Dynamic FAQ Handler #126
Dynamic FAQ Handler #126
Conversation
WalkthroughReplaces the static FAQ handler with a site-restricted web-search + LLM pipeline, updates call sites to pass Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DevRelAgent
participant ReactSupervisor
participant FAQHandler
participant SearchTool
participant LLM
User->>DevRelAgent: Ask org-level question
DevRelAgent->>ReactSupervisor: react_supervisor_node(state)
ReactSupervisor-->>DevRelAgent: Decision: faq_handler
DevRelAgent->>FAQHandler: handle_faq_node(state, search_tool, llm)
FAQHandler->>LLM: Refine query (with org/site filters)
LLM-->>FAQHandler: Refined query
FAQHandler->>SearchTool: Web search (site_filters)
SearchTool-->>FAQHandler: Search results
FAQHandler->>LLM: Synthesize answer + sources
LLM-->>FAQHandler: Synthesized answer
FAQHandler-->>DevRelAgent: Formatted response + sources
DevRelAgent-->>User: Answer with citations
sequenceDiagram
participant DevRelAgent
participant ReactSupervisor
participant LLM
DevRelAgent->>ReactSupervisor: react_supervisor_node(state)
ReactSupervisor->>ReactSupervisor: Validate state, enforce caps
ReactSupervisor->>LLM: Supervisor prompt (context + tool_results)
LLM-->>ReactSupervisor: THINK / ACT / REASON
ReactSupervisor->>ReactSupervisor: Parse & validate action (guarded)
ReactSupervisor-->>DevRelAgent: Decision, updated iteration_count or completion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (9)
env.example (1)
29-29: Trailing whitespace on MAX_RETRIESRemove the trailing space to avoid lint churn.
-MAX_RETRIES=3 +MAX_RETRIES=3backend/app/agents/devrel/nodes/handlers/faq.py (2)
114-181: Prompts: lower log verbosity for user content and queriesSensitive user content and generated prompts are logged at INFO. Consider downgrading to DEBUG to reduce PII exposure in logs.
- logger.info(f"[FAQ_HANDLER] Refined query: {refined_query}") + logger.debug(f"[FAQ_HANDLER] Refined query: {refined_query}") ... - logger.info("[FAQ_HANDLER] Step 3-4: Synthesizing search results into FAQ response") + logger.debug("[FAQ_HANDLER] Step 3-4: Synthesizing search results into FAQ response") ... - logger.info(f"[FAQ_HANDLER] Synthesized FAQ response: {synthesized_answer[:100]}...") + logger.debug(f"[FAQ_HANDLER] Synthesized FAQ response: {synthesized_answer[:100]}...")
214-238: Inline test harness in production moduleThe main test harness is useful locally but should live under tests/ to avoid accidental execution and to keep modules import-safe.
-# Example usage for testing -if __name__ == "__main__": - ... - asyncio.run(test_faq_handler()) +# Consider moving the inline test harness to tests/ with an async test case.backend/app/agents/devrel/nodes/react_supervisor.py (6)
167-171: Use timezone-aware timestamps in tool resultsNaive datetime.now() is ambiguous; prefer UTC-aware timestamps for consistent logs and auditing.
Apply this diff:
-from datetime import datetime +from datetime import datetime, timezone @@ - "timestamp": datetime.now().isoformat() + "timestamp": datetime.now(timezone.utc).isoformat()Also applies to: 3-3
179-187: Deduplicate tools_used and handle None robustlyAvoid unbounded duplicates in tools_used and guard against None.
Apply this diff:
- tools_used = getattr(state, 'tools_used', []) + [tool_name] + existing_tools = getattr(state, 'tools_used', []) or [] + tools_used = list(dict.fromkeys([*existing_tools, tool_name]))
85-114: Make THINK/ACT/REASON parsing case-insensitive and tolerant to format variationsCurrent parsing requires exact uppercase keywords with no space before colon. LLMs often vary casing/spacing (e.g., "Act:", "REASON :"). Normalize case and split on the first colon.
Apply this diff:
- for line in response.strip().split("\n"): - line = line.strip() + for line in response.strip().split("\n"): + line = line.strip() + line_upper = line.upper() if not line: continue @@ - if line.startswith("THINK:"): + if line_upper.startswith("THINK"): if current_section and content_buffer: decision[current_section] = " ".join(content_buffer) current_section = "thinking" - content_buffer = [line.replace("THINK:", "").strip()] + content_buffer = [line.split(":", 1)[1].strip()] if ":" in line else [] @@ - elif line.startswith("ACT:"): + elif line_upper.startswith("ACT"): if current_section and content_buffer: decision[current_section] = " ".join(content_buffer) - action = line.replace("ACT:", "").strip().lower() + action = line.split(":", 1)[1].strip().lower() if ":" in line else "" if action in VALID_ACTIONS: decision["action"] = action else: logger.warning(f"Invalid action '{action}', defaulting to 'complete'") decision["action"] = "complete" current_section = None content_buffer = [] @@ - elif line.startswith("REASON:"): + elif line_upper.startswith("REASON"): if current_section and content_buffer: decision[current_section] = " ".join(content_buffer) current_section = "reasoning" - content_buffer = [line.replace("REASON:", "").strip()] + content_buffer = [line.split(":", 1)[1].strip()] if ":" in line else [] @@ - if decision["action"] not in VALID_ACTIONS: + if decision["action"] not in VALID_ACTIONS: logger.warning(f"Final validation failed for action '{decision['action']}', defaulting to 'complete'") decision["action"] = "complete"Also applies to: 120-123
227-238: Strengthen state validation (types) to fail fast on malformed stateMinimal checks are good, but validating types prevents subtle downstream errors.
Apply this diff:
try: if not state: return False if not getattr(state, 'session_id', None): logger.error("Invalid state: missing session_id") return False - if not hasattr(state, 'context'): + if not hasattr(state, 'context'): logger.error("Invalid state: missing context") return False + if not isinstance(state.context, dict): + logger.error("Invalid state: context must be a dict") + return False + if hasattr(state, 'messages') and state.messages is not None and not isinstance(state.messages, list): + logger.error("Invalid state: messages must be a list") + return False return True
57-66: Consider not storing raw supervisor_thinking unless in debug modeStoring full LLM internal thoughts can leak sensitive content and inflate state size. Keep decision, but gate raw "supervisor_thinking" behind a debug flag or truncate.
If you want to keep it, consider:
- Truncating to N chars (e.g., 1k).
- Only storing when an env var like DEVREL_DEBUG is true.
Also applies to: 244-273
73-128: Add focused unit tests for decision parsing, iteration bounds, and tool result mergingThese flows are critical to routing correctness and robustness.
Proposed test coverage:
- _parse_supervisor_decision handles: empty response, mixed-case tags, extra spaces, invalid action fallback, multi-line THINK/REASON accumulation.
- react_supervisor_node iteration cap triggers completion when iteration_count >= MAX_ITERATIONS.
- supervisor_decision_router respects the same bound (>=) and defaults to "complete" on invalid actions.
- add_tool_result trims history to 20, uses UTC timestamps, and deduplicates tools_used.
I can draft pytest tests if helpful. Would you like me to open a new issue or add a test file in this PR?
Also applies to: 130-151, 153-189
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/app/agents/devrel/agent.py(1 hunks)backend/app/agents/devrel/nodes/handlers/faq.py(1 hunks)backend/app/agents/devrel/nodes/react_supervisor.py(1 hunks)backend/app/agents/devrel/tool_wrappers.py(1 hunks)backend/app/agents/devrel/tools/faq_tool.py(1 hunks)env.example(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
backend/app/agents/devrel/agent.py (2)
backend/app/agents/devrel/tool_wrappers.py (1)
faq_handler_tool_node(18-24)backend/app/services/embedding_service/service.py (1)
llm(55-68)
backend/app/agents/devrel/nodes/react_supervisor.py (2)
backend/app/agents/state.py (1)
AgentState(18-73)backend/app/agents/devrel/nodes/generate_response.py (1)
_get_latest_message(97-101)
backend/app/agents/devrel/nodes/handlers/faq.py (4)
backend/app/agents/state.py (1)
AgentState(18-73)backend/app/services/embedding_service/service.py (1)
llm(55-68)backend/app/agents/devrel/tools/faq_tool.py (1)
search(35-37)backend/app/agents/base_agent.py (1)
run(22-54)
backend/app/agents/devrel/tool_wrappers.py (5)
backend/app/agents/state.py (1)
AgentState(18-73)backend/app/services/embedding_service/service.py (1)
llm(55-68)backend/app/agents/devrel/nodes/handlers/faq.py (1)
handle_faq_node(27-59)backend/app/agents/devrel/nodes/react_supervisor.py (1)
add_tool_result(153-192)backend/app/agents/devrel/nodes/handlers/onboarding.py (1)
handle_onboarding_node(6-17)
🔇 Additional comments (4)
backend/app/agents/devrel/tools/faq_tool.py (1)
43-43: Fuzzy-match simplification LGTMThe inlined intersection check is concise and equivalent.
backend/app/agents/devrel/tool_wrappers.py (1)
37-41: Minor readability improvement LGTMThe latest_message one-liner is concise and clear.
backend/app/agents/devrel/nodes/handlers/faq.py (1)
87-106: Ignore string‐shape normalization; search always returns a list of dicts
The currentDuckDuckGoSearchTool.searchimplementation (and any other active tool) always returns aList[Dict[str, Any]](or an empty list on error). There is no code path that yields a raw string or single dict, so the added normalization isn’t needed.Likely an incorrect or invalid review comment.
backend/app/agents/devrel/nodes/react_supervisor.py (1)
130-147: Action Name Consistency ConfirmedAll routing action strings are used uniformly across the codebase, so there are no dead routes:
- VALID_ACTIONS in backend/app/agents/devrel/nodes/react_supervisor.py
- Workflow mapping in backend/app/agents/devrel/agent.py (lines 66–69)
- Tool wrappers in backend/app/agents/devrel/tool_wrappers.py
- Prompt definitions in backend/app/agents/devrel/prompts/react_prompt.py
No changes required.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
env.example (1)
31-35: Broken interpolation: ORG_ vars are not defined; OFFICIAL_HANDLE_ will expand to empty/literal strings**Define ORG_NAME and ORG_WEBSITE/ORG_GITHUB/ORG_TWITTER before referencing them. Otherwise, the FAQ handler may generate queries like site:${ORG_WEBSITE} or no site filters at all.
Apply this diff:
# Agent Configuration (optional) DEVREL_AGENT_MODEL=gemini-2.5-flash GITHUB_AGENT_MODEL=gemini-2.5-flash CLASSIFICATION_AGENT_MODEL=gemini-2.0-flash AGENT_TIMEOUT=30 -MAX_RETRIES=3 +MAX_RETRIES=3 -# Explicit handles for faq.py +# Organization identity and official sources (consumed by Settings and FAQ handler) +ORG_NAME=AOSSIE +ORG_WEBSITE=https://aossie.org +ORG_GITHUB=https://github.com/AOSSIE-Org +ORG_TWITTER=https://twitter.com/aossie_org + +# Explicit handles for faq.py (map to ORG_* above) OFFICIAL_HANDLE_1=${ORG_WEBSITE} OFFICIAL_HANDLE_2=${ORG_GITHUB} OFFICIAL_HANDLE_3=${ORG_TWITTER}backend/app/agents/devrel/nodes/handlers/faq.py (1)
13-16: Good: removed import-time logging.basicConfigAvoiding global logging configuration in a library module is the right call. Let the application configure logging.
backend/app/agents/devrel/nodes/react_supervisor.py (1)
61-66: Good: guard len() against None latest_messageUsing len(latest_message or "") avoids TypeError.
🧹 Nitpick comments (5)
env.example (1)
29-29: Nit: trailing space after MAX_RETRIESTrim the trailing space to avoid accidental parsing issues.
-MAX_RETRIES=3 +MAX_RETRIES=3backend/app/agents/devrel/nodes/handlers/faq.py (3)
53-69: Site filters: robust domain extraction; consider native include_domains when availableDomain parsing with urlparse is solid. Two improvement suggestions:
- Prefer passing include_domains (or equivalent) to the search tool if supported, falling back to query-time site: tokens only when necessary.
- For broad engine compatibility, concatenating site filters with whitespace is usually sufficient; the literal AND inside the query can be brittle depending on the backend parser.
If you want, I can update the wrapper/tool to optionally accept include_domains and branch accordingly.
108-112: Query composition: avoid brittle "AND" in the free-text stringSome providers do not honor boolean operators in the query string. Safer composition:
- if site_filters: - refined_query = f"({refined_query}) AND ({site_filters})" + if site_filters: + refined_query = f"{refined_query} {site_filters}"
241-264: Keep test harness out of production modulesThe main AsyncMock harness is handy, but consider moving this to a proper test under tests/ with pytest-asyncio to keep modules clean.
backend/app/agents/devrel/nodes/react_supervisor.py (1)
186-191: Prefer timezone-aware timestampsUse UTC ISO timestamps to avoid ambiguity.
-from datetime import datetime +from datetime import datetime, timezone @@ - "timestamp": datetime.now().isoformat() + "timestamp": datetime.now(timezone.utc).isoformat()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/app/agents/devrel/agent.py(1 hunks)backend/app/agents/devrel/nodes/handlers/faq.py(1 hunks)backend/app/agents/devrel/nodes/react_supervisor.py(1 hunks)backend/app/agents/devrel/tool_wrappers.py(1 hunks)backend/app/agents/devrel/tools/faq_tool.py(1 hunks)backend/app/core/config/settings.py(1 hunks)env.example(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/app/agents/devrel/tools/faq_tool.py
- backend/app/agents/devrel/tool_wrappers.py
- backend/app/agents/devrel/agent.py
🧰 Additional context used
🪛 Ruff (0.12.2)
backend/app/agents/devrel/nodes/react_supervisor.py
180-180: Local variable tool_results_str is assigned to but never used
Remove assignment to unused variable tool_results_str
(F841)
🔇 Additional comments (5)
backend/app/agents/devrel/nodes/handlers/faq.py (2)
41-52: Nice early-exit guard for empty messageGood defensive check and graceful fallback without invoking the LLM or search.
216-221: Sanitize titles/URLs downstream to prevent UI XSSSince titles/urls come from the web, ensure the renderer escapes markdown/html. If you control the UI, sanitize before rendering.
backend/app/agents/devrel/nodes/react_supervisor.py (3)
33-37: Iteration cap guard in node looks goodNode prevents generating decisions once the cap is hit. Keep as-is.
38-47: Good: safely serializing tool_results for prompt constructionUsing default=str and fallback prevents crashes from non-serializable entries.
Also applies to: 54-55
199-208: Actionable Directive: Verify State Merger’s Handling oftools_usedTo confirm whether top‐level
tools_usedis merged intoAgentState.tools_usedor ignored, please locate and inspect the code that applies a node’s return-dict to theAgentState. For example, search for whereoutput = node.run(...)is consumed and how it updatesstate.contextandstate.tools_used.Suggested search commands:
# Find where node outputs are merged into AgentState rg -n "state.context" -n backend/app/agents rg -n "tools_used" -n backend/app/agents rg -n "AgentState" -n backend/app/agentsOnce you’ve identified whether
tools_usedmust be returned at top level or embedded incontext, updatereact_supervisor.pyaccordingly:If the merger only looks at
context:- tools_used = getattr(state, 'tools_used', []) + [tool_name] - - return { - "context": { - **state.context, - "tool_results": tool_results - }, - "tools_used": tools_used, - "current_task": f"completed_{tool_name}" - } + tools_used = state.context.get("tools_used", []) + [tool_name] + + return { + "context": { + **state.context, + "tool_results": tool_results, + "tools_used": tools_used + }, + "current_task": f"completed_{tool_name}" + }If the merger respects top-level
tools_used, no change is needed.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
backend/app/agents/devrel/nodes/handlers/faq.py (3)
5-5: Centralized org settings usage looks goodSwitching to app_settings and dropping ad-hoc env parsing aligns config to a single source of truth. Good cleanup.
Also applies to: 12-15
8-9: Good: no global logging configuration at import timeUsing a module logger without basicConfig avoids clobbering app-level logging config.
30-40: Early exit on empty user messageSolid guardrail to avoid unnecessary LLM/search calls and return a helpful fallback.
🧹 Nitpick comments (6)
backend/app/agents/devrel/nodes/handlers/faq.py (6)
42-57: Tighten site scoping for GitHub/Twitter using inurl to reduce noiseCurrent site filters restrict by domain only (e.g., github.com), which can be very broad. Optionally add inurl constraints for handles where a subpath is known (GitHub org, Twitter handle) to bias results toward the org space while still staying provider-agnostic.
Apply this diff:
- from urllib.parse import urlparse - domains = [] + from urllib.parse import urlparse + filters = set() for u in OFFICIAL_HANDLES: try: parsed = urlparse(u) - domain = parsed.netloc or parsed.path # handles bare domains - if domain: - domains.append(domain) + domain = parsed.netloc or parsed.path # handles bare domains + if not domain: + continue + filters.add(f"site:{domain}") + path = (parsed.path or "").strip("/") + # Add path scoping for common hubs to reduce noise + if path and (domain.endswith("github.com") or domain.endswith("twitter.com")): + filters.add(f"site:{domain} inurl:{path}") except Exception: continue - site_filters = " OR ".join([f"site:{d}" for d in domains]) + site_filters = " OR ".join(filters)
192-195: Reduce verbosity and potential leakage in logsLog synthesized content preview at debug, not info.
Apply this diff:
- logger.info(f"[FAQ_HANDLER] Synthesized FAQ response: {synthesized_answer[:100]}...") + logger.debug(f"[FAQ_HANDLER] Synthesized FAQ response: {synthesized_answer[:100]}...")
67-74: Consider returning structured sources alongside the formatted answerFor richer UIs, expose sources as structured metadata (e.g., array of {title, url}) in task_result rather than only embedding Markdown links in the string. This enables downstream rendering and analytics without parsing.
Follow-up: have _dynamic_faq_process return both text and the top-N sources, or compute sources in handle_faq_node before returning task_result.
17-21: Optional: add lightweight Protocols for search_tool and llmHelps static checking and refactoring safety without tying to specific implementations.
Add near the imports:
from typing import Protocol, runtime_checkable, Sequence from langchain_core.messages import BaseMessage @runtime_checkable class SupportsSearch(Protocol): async def search(self, query: str) -> List[Dict[str, Any]]: ... @runtime_checkable class SupportsChatInvoke(Protocol): async def ainvoke(self, messages: Sequence[BaseMessage]) -> Any: ...Then annotate:
async def handle_faq_node(state: AgentState, search_tool: SupportsSearch, llm: SupportsChatInvoke) -> dict: ... async def _dynamic_faq_process(..., search_tool: SupportsSearch, llm: SupportsChatInvoke, ...): ...
229-253: Move module-level test harness to proper testsKeep modules import-safe. Relocate main harness to tests or a sample notebook/script.
Apply this diff to remove it here:
-# Example usage for testing -if __name__ == "__main__": - import asyncio - from unittest.mock import AsyncMock - - class MockState: - session_id = "test_session" - messages = [{"content": "What projects does your organization have?"}] - context = {} - - async def test_faq_handler(): - mock_state = MockState() - mock_search_tool = AsyncMock() - mock_search_tool.search.return_value = [ - {"title": "Project A", "content": "Details about Project A.", "url": "https://aossie.org/projects/a"}, - {"title": "Project B", "content": "Details about Project B.", "url": "https://aossie.org/projects/b"}, - ] - mock_llm = AsyncMock() - mock_llm.ainvoke.return_value = AsyncMock(content="We have Project A and Project B focusing on AI and Web.") - - response = await handle_faq_node(mock_state, mock_search_tool, mock_llm) - print("FAQ Handler response:") - print(response) - - asyncio.run(test_faq_handler())
97-101: Make query composition provider-agnostic by using whitespace instead of explicit ANDUpdate the FAQ handler’s query concatenation to avoid hard-coding
AND, relying on implicit AND (whitespace) which is supported more broadly:--- a/backend/app/agents/devrel/nodes/handlers/faq.py +++ b/backend/app/agents/devrel/nodes/handlers/faq.py @@ -98,7 +98,7 @@ async def _dynamic_faq_process( - if site_filters: - refined_query = f"({refined_query}) AND ({site_filters})" + if site_filters: + refined_query = f"{refined_query} {site_filters}" logger.info(f"[FAQ_HANDLER] Refined and filtered query: {refined_query}")This change removes the explicit
AND, allowing providers that treat whitespace as an implicit AND to handle the query correctly. Please verify that each search tool implementation you use (e.g. DuckDuckGoSearchTool, TavilySearchTool) supports boolean operators and grouping as expected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/app/agents/devrel/nodes/handlers/faq.py(1 hunks)backend/app/agents/devrel/nodes/react_supervisor.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/agents/devrel/nodes/react_supervisor.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/app/agents/devrel/nodes/handlers/faq.py (3)
backend/app/agents/state.py (1)
AgentState(18-73)backend/app/services/embedding_service/service.py (1)
llm(55-68)backend/app/agents/base_agent.py (1)
run(22-54)
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
backend/app/core/config/settings.py (1)
55-58: Pydantic v2: class Config is ignored; use SettingsConfigDictIn pydantic v2/pydantic-settings v2, the nested Config class is no-op. Switch to model_config so env_file/extra/case sensitivity actually take effect.
Apply this diff:
+from pydantic_settings import SettingsConfigDict @@ - class Config: - env_file = ".env" - extra = "ignore" # to prevent errors from extra env variables + # pydantic-settings v2 configuration + model_config = SettingsConfigDict( + env_file=".env", + extra="ignore", + case_sensitive=False, # allow ORG_NAME or org_name + )
🧹 Nitpick comments (1)
backend/app/core/config/settings.py (1)
42-42: Nit: remove emoji from code comments for consistencyAvoid emojis in source comments to keep style consistent with the rest of the file.
Apply this diff:
- # ✅ New org info fields (pulled from .env automatically) + # Organization info fields (populated from environment)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/core/config/settings.py(1 hunks)
🔇 Additional comments (1)
backend/app/core/config/settings.py (1)
42-47: LGTM: centralizing org identity in SettingsAdding org_* fields here aligns with the goal to make branding/handles a single source of truth for the dynamic FAQ flow. This addresses prior feedback about avoiding separate env parsing in faq.py.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
backend/app/agents/devrel/nodes/handlers/faq.py (1)
42-58: Normalize handles to bare domains for reliable site: filteringAs written, Line 49 may capture path segments (e.g., "github.com/AOSSIE-Org") when OFFICIAL_HANDLES contain bare values without a scheme, which would lead to invalid site: filters. Strip path segments and "www." to ensure domains only.
Apply this diff within the loop:
- domain = parsed.netloc or parsed.path # handles bare domains - if domain: - domains.append(domain) + domain = parsed.netloc or parsed.path # handles bare domains + if domain: + # Keep only host, drop any path and "www." prefix + domain = domain.split("/")[0] + if domain.startswith("www."): + domain = domain[4:] + if domain: + domains.append(domain)Optionally dedupe before building filters:
- site_filters = " OR ".join([f"site:{d}" for d in domains]) + domains = list(dict.fromkeys(domains)) # preserve order, remove dups + site_filters = " OR ".join([f"site:{d}" for d in domains])
🧹 Nitpick comments (4)
backend/app/agents/devrel/nodes/handlers/faq.py (4)
24-28: Make sure you’re using the latest human/user message, not potentially the last assistant messageIf state.messages ends with an assistant reply, Line 26 may pick up the assistant’s content. Prefer selecting the most recent human/user message.
If your messages include a role/type, consider:
- if state.messages: - latest_message = state.messages[-1].get("content", "") + if state.messages: + for msg in reversed(state.messages): + role = (msg.get("role") or msg.get("type") or "user").lower() + if role in ("user", "human"): + latest_message = msg.get("content", "") + breakPlease confirm whether your message dicts include role/type so we can finalize the patch accordingly.
200-213: Channel-aware formatting for sourcesThe Markdown/emoji formatting on Lines 207-212 is nice but may render poorly on some channels. Consider a settings toggle (e.g., faq_markdown_enabled) or falling back to plain text when markdown is unsupported.
For a plain-text fallback:
- formatted_response += "\n\n**📚 Sources:**" + formatted_response += "\n\nSources:" - formatted_response += f"\n{i+1}. [{title}]({url})" + formatted_response += f"\n{i+1}. {title} - {url}"
231-255: Move the main test harness to a proper test to keep production modules cleanKeeping executable harnesses in production modules is unconventional and easy to forget. Move this to tests/ with AsyncMock-based unit tests.
I can port this into pytest/asyncio tests that cover:
- Empty-message fallback
- Successful dynamic flow (query refinement, search, synthesis)
- Search error path
- No-results path
- Source formatting cap (top 3)
Want me to push a test module?
17-21: Stronger typing via Protocols for search_tool and llmUsing Any everywhere weakens static guarantees. Introduce minimal Protocols to document the expected async interface.
Add (outside the selected range):
from typing import Protocol, runtime_checkable @runtime_checkable class AsyncSearchTool(Protocol): async def search(self, query: str) -> List[Dict[str, Any]]: ... @runtime_checkable class AsyncLLM(Protocol): async def ainvoke(self, messages: List[HumanMessage]) -> Any: ...Then:
-async def handle_faq_node(state: AgentState, search_tool: Any, llm: Any) -> dict: +async def handle_faq_node(state: AgentState, search_tool: AsyncSearchTool, llm: AsyncLLM) -> dict:This keeps flexibility while improving correctness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/agents/devrel/nodes/handlers/faq.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/app/agents/devrel/nodes/handlers/faq.py (3)
backend/app/agents/state.py (1)
AgentState(18-73)backend/app/services/embedding_service/service.py (1)
llm(55-68)backend/app/agents/base_agent.py (1)
run(22-54)
🔇 Additional comments (4)
backend/app/agents/devrel/nodes/handlers/faq.py (4)
8-10: Good: Logger configured without global basicConfigModule uses a local logger and avoids configuring global logging at import-time. This aligns with best practices and prior review feedback.
12-15: Good: Centralized org config via app settingsUsing app_settings for ORG_NAME and OFFICIAL_HANDLES prevents env drift and consolidates configuration.
30-41: Good: Early exit on empty message with graceful fallbackShort-circuiting avoids unnecessary tool/LLM invocations and returns a structured fallback.
97-101: Verify search_tool’s Boolean operator supportPlease confirm whether the
search_tool.searchimplementation reliably handles explicit Boolean operators (e.g., “AND”). Many web-search back ends ignore or misinterpret these operators, which can lead to unexpected results. If it does not:
- In
backend/app/agents/devrel/nodes/handlers/faq.py, inside_dynamic_faq_process(lines ~97–101), replacewith a more search-friendly concatenation, for example:if site_filters: refined_query = f"({refined_query}) AND ({site_filters})"if site_filters: refined_query = f"{refined_query} {site_filters}"- Better yet, if your search tool exposes a structured API (e.g.,
search_tool.search(query, site_filters=domains)), switch to that instead of in-query filtering.
| logger.info(f"[FAQ_HANDLER] Step 1: Refining FAQ query for org '{org_name}'") | ||
| refined_query = await _refine_faq_query(message, llm, org_name) | ||
|
|
||
| # Append site filters for restricting to official handles | ||
| if site_filters: | ||
| refined_query = f"({refined_query}) AND ({site_filters})" | ||
| logger.info(f"[FAQ_HANDLER] Refined and filtered query: {refined_query}") | ||
|
|
There was a problem hiding this comment.
Reduce user-content logging to DEBUG and truncate to lower PII exposure and noise
Refined queries and synthesized responses may contain user-provided content. Logging them at INFO can leak PII and increase noise.
Apply:
- logger.info(f"[FAQ_HANDLER] Step 1: Refining FAQ query for org '{org_name}'")
+ logger.debug(f"[FAQ_HANDLER] Step 1: Refining FAQ query for org '{org_name}'")
@@
- logger.info(f"[FAQ_HANDLER] Refined and filtered query: {refined_query}")
+ logger.debug(f"[FAQ_HANDLER] Refined and filtered query: {refined_query[:200]}{'...' if len(refined_query) > 200 else ''}")
@@
- logger.info(f"[FAQ_HANDLER] Synthesized FAQ response: {synthesized_answer[:100]}...")
+ logger.debug(f"[FAQ_HANDLER] Synthesized FAQ response: {synthesized_answer[:100]}...")If you need INFO-level observability, consider redacting with a sanitizer (emails, phone numbers) before logging.
Also applies to: 196-196
| logger.info(f"[FAQ_HANDLER] Step 2: Searching for: {refined_query}") | ||
| try: | ||
| search_results = await search_tool.search(refined_query) | ||
| except Exception as search_err: | ||
| logger.error(f"[FAQ_HANDLER] Search tool error: {search_err}") | ||
| return _generate_fallback_response(message, org_name) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add timeouts around external search calls to prevent hangs
Wrap the search call with asyncio.wait_for using configurable timeouts; otherwise a network stall can block the node.
Apply:
- try:
- search_results = await search_tool.search(refined_query)
+ try:
+ search_results = await asyncio.wait_for(
+ search_tool.search(refined_query),
+ timeout=FAQ_SEARCH_TIMEOUT
+ )Add these near the top of the module (outside the selected range):
import asyncio # at the imports
# Timeouts with safe defaults; override via settings if available
FAQ_SEARCH_TIMEOUT = getattr(app_settings, "faq_search_timeout", 10.0)
FAQ_LLM_TIMEOUT = getattr(app_settings, "faq_llm_timeout", 15.0)🤖 Prompt for AI Agents
In backend/app/agents/devrel/nodes/handlers/faq.py around lines 103 to 108, the
external search call can hang; add an asyncio timeout: import asyncio at the top
and define FAQ_SEARCH_TIMEOUT = getattr(app_settings, "faq_search_timeout",
10.0) (and FAQ_LLM_TIMEOUT similarly if present), then replace the direct await
search_tool.search(refined_query) with await
asyncio.wait_for(search_tool.search(refined_query), FAQ_SEARCH_TIMEOUT); catch
asyncio.TimeoutError (and still catch general Exception) and log a
timeout-specific error before returning _generate_fallback_response(message,
org_name).
PrithvijitBose
force-pushed
the
FAQ-HANDLER
branch
4 times, most recently
from
fe0cfe8 to
de1246e
Compare
… empty message fallback
…r) with env var aliases
PrithvijitBose
force-pushed
the
FAQ-HANDLER
branch
from
de1246e to
b05f020
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
backend/app/agents/devrel/nodes/react_supervisor.py (2)
34-37: Fix iteration cap check to allow exactly MAX_ITERATIONSThe current check prevents routing the action decided on the MAX-th iteration. Since the node increments
iteration_countafter decision-making, you should use>instead of>=to allow exactly MAX_ITERATIONS to proceed.- if iteration_count >= MAX_ITERATIONS: + if iteration_count > MAX_ITERATIONS: logger.warning(f"Max iterations ({MAX_ITERATIONS}) reached for session {state.session_id}") return _create_completion_response(state, "Maximum iterations reached")
147-149: Align iteration boundary check with node logicThis router check should match the node's iteration check. Since you're incrementing before routing, use
>to allow the MAX-th iteration's action to be routed.- if iteration_count > MAX_ITERATIONS: + if iteration_count >= MAX_ITERATIONS: logger.warning(f"Max iterations reached for session {state.session_id}") return "complete"Wait, I notice the current code already uses
>which is actually correct. The node should be changed to match this, not the other way around.backend/app/agents/devrel/nodes/handlers/faq.py (1)
103-108: Add timeout for search operationsExternal search calls could hang indefinitely. Consider adding a timeout to prevent blocking.
Add at the top of the file:
import asyncio # Configurable timeouts FAQ_SEARCH_TIMEOUT = getattr(app_settings, "faq_search_timeout", 10.0) FAQ_LLM_TIMEOUT = getattr(app_settings, "faq_llm_timeout", 15.0)Then wrap the search call:
try: - search_results = await search_tool.search(refined_query) + search_results = await asyncio.wait_for( + search_tool.search(refined_query), + timeout=FAQ_SEARCH_TIMEOUT + ) + except asyncio.TimeoutError: + logger.error(f"[FAQ_HANDLER] Search timeout after {FAQ_SEARCH_TIMEOUT}s") + return _generate_fallback_response(message, org_name) except Exception as search_err:
🧹 Nitpick comments (2)
backend/app/agents/devrel/nodes/handlers/faq.py (2)
94-100: Consider adding DEBUG level for detailed query loggingThe refined query may contain user-provided content. Consider using DEBUG level and truncating for privacy.
- logger.info(f"[FAQ_HANDLER] Step 1: Refining FAQ query for org '{org_name}'") + logger.debug(f"[FAQ_HANDLER] Step 1: Refining FAQ query for org '{org_name}'") # ... - logger.info(f"[FAQ_HANDLER] Refined and filtered query: {refined_query}") + logger.debug(f"[FAQ_HANDLER] Refined and filtered query: {refined_query[:200]}{'...' if len(refined_query) > 200 else ''}")
232-255: Consider moving test code to a separate test fileThe test harness in
__main__is useful for development but should ideally be in a separate test file for better organization.Consider moving this to a file like
tests/test_faq_handler.pyto keep the production code clean and maintain separation of concerns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/app/agents/devrel/agent.py(1 hunks)backend/app/agents/devrel/nodes/handlers/faq.py(1 hunks)backend/app/agents/devrel/nodes/react_supervisor.py(1 hunks)backend/app/agents/devrel/tool_wrappers.py(1 hunks)backend/app/agents/devrel/tools/faq_tool.py(1 hunks)backend/app/core/config/settings.py(2 hunks)env.example(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- backend/app/agents/devrel/agent.py
- backend/app/agents/devrel/tools/faq_tool.py
- env.example
- backend/app/core/config/settings.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-08T13:31:11.572Z
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#72
File: backend/app/agents/devrel/nodes/handle_web_search_node.py:31-42
Timestamp: 2025-06-08T13:31:11.572Z
Learning: In backend/app/agents/devrel/tools/search_tool.py, the TavilySearchTool.search() method has partial error handling for missing API key, AttributeError, ConnectionError, and TimeoutError, but lacks a comprehensive Exception catch-all block, so calling functions may still need additional error handling for other potential exceptions.
Applied to files:
backend/app/agents/devrel/nodes/handlers/faq.py
🧬 Code Graph Analysis (3)
backend/app/agents/devrel/nodes/handlers/faq.py (3)
backend/app/agents/state.py (1)
AgentState(18-73)backend/app/services/embedding_service/service.py (1)
llm(55-68)backend/app/agents/base_agent.py (1)
run(22-54)
backend/app/agents/devrel/nodes/react_supervisor.py (2)
backend/app/agents/state.py (1)
AgentState(18-73)backend/app/agents/devrel/nodes/generate_response.py (1)
_get_latest_message(97-101)
backend/app/agents/devrel/tool_wrappers.py (5)
backend/app/agents/state.py (1)
AgentState(18-73)backend/app/services/embedding_service/service.py (1)
llm(55-68)backend/app/agents/devrel/nodes/handlers/faq.py (1)
handle_faq_node(17-74)backend/app/agents/devrel/nodes/react_supervisor.py (1)
add_tool_result(163-202)backend/app/agents/devrel/nodes/handlers/onboarding.py (1)
handle_onboarding_node(6-17)
🔇 Additional comments (12)
backend/app/agents/devrel/nodes/react_supervisor.py (4)
62-62: Good defensive programming for None handlingThe use of
len(latest_message or '')properly guards againstNonevalues that could cause a TypeError.
205-212: Good error handling wrapper for utility functionThe wrapper properly catches exceptions from the imported utility function and provides a safe fallback, preventing crashes in the supervisor node.
237-252: Comprehensive state validationThe validation function properly checks for essential state attributes with detailed error logging, which will help with debugging issues in production.
38-46: Remove unused variabletool_results_strThe variable
tool_results_stris computed but never used in the prompt construction. The prompt uses the same serialization logic directly on line 54.- # Safely serialize tool_results for prompt usage - try: - if tool_results: - tool_results_str = json.dumps(tool_results, indent=2, default=str) - else: - tool_results_str = "No previous tool results" - except Exception as e: - logger.warning(f"Failed to serialize tool_results: {e}") - tool_results_str = str(tool_results) if tool_results else "No previous tool results"Then update line 54 to safely serialize:
- tool_results=tool_results_str + tool_results=json.dumps(tool_results, indent=2, default=str) if tool_results else "No previous tool results"Likely an incorrect or invalid review comment.
backend/app/agents/devrel/tool_wrappers.py (3)
18-24: LGTM! Wrapper signature properly aligned with FAQ handlerThe function signature correctly accepts
search_toolandllmparameters and forwards them to the FAQ handler, which aligns with the dynamic FAQ implementation requirements.
37-41: Good refactoring for message extractionThe conditional expression cleanly handles both scenarios (messages present vs context fallback) in a readable way.
43-53: Excellent error handling for GitHub toolkitThe try-catch block properly captures toolkit errors and returns a structured error response that can be processed downstream, preventing crashes while maintaining observability.
backend/app/agents/devrel/nodes/handlers/faq.py (5)
13-14: Good use of centralized settingsUsing
app_settingsfor organization configuration ensures consistency across the application and avoids duplicate environment variable parsing.
30-41: Smart early exit for empty messagesThe early return prevents unnecessary LLM and search API calls when there's no user input to process, improving efficiency.
43-57: Well-implemented domain extraction for site filtersThe URL parsing logic properly extracts domains from URLs and handles both full URLs and bare domains, with good error handling for malformed URLs.
169-171: Good content truncation for token managementLimiting content to 500 characters prevents token explosion in LLM prompts while maintaining useful context.
216-228: Excellent user-friendly fallback messageThe fallback response is informative, explains potential reasons for failure, and provides actionable next steps for users.
| response = await llm.ainvoke([HumanMessage(content=synthesis_prompt)]) | ||
| synthesized_answer = response.content.strip() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply timeout to LLM synthesis calls
Similar to search operations, LLM calls should have timeouts to prevent hanging.
- response = await llm.ainvoke([HumanMessage(content=synthesis_prompt)])
+ response = await asyncio.wait_for(
+ llm.ainvoke([HumanMessage(content=synthesis_prompt)]),
+ timeout=FAQ_LLM_TIMEOUT
+ )Add exception handling:
except asyncio.TimeoutError:
logger.error(f"[FAQ_HANDLER] LLM timeout after {FAQ_LLM_TIMEOUT}s")
return _generate_fallback_response(message, org_name)🤖 Prompt for AI Agents
In backend/app/agents/devrel/nodes/handlers/faq.py around lines 194-195, wrap
the LLM synthesis call in asyncio.wait_for using the FAQ_LLM_TIMEOUT constant
and add exception handling: put the await llm.ainvoke(...) inside a try block
with response = await asyncio.wait_for(llm.ainvoke(...), FAQ_LLM_TIMEOUT), catch
asyncio.TimeoutError, log the timeout with logger.error(f"[FAQ_HANDLER] LLM
timeout after {FAQ_LLM_TIMEOUT}s") and return
_generate_fallback_response(message, org_name); ensure asyncio is imported if
not already.
There was a problem hiding this comment.
The workflow seems fine to me. A lot of restructuring changes are needed. Please do the needful.
I'm up for suggestions on your implementation.
Thanks a lot @PrithvijitBose !
PS: Please do review the coderrabit suggestions too.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/app/agents/devrel/tool_wrappers.py (1)
18-24: LGTM! Signature correctly aligned with dynamic FAQ handler.The parameter rename from
faq_tooltosearch_tooland the updated call tohandle_faq_nodecorrectly implement the past review feedback. This allows the wrapper to acceptDuckDuckGoSearchTooland forward it to the dynamic FAQ handler.
🧹 Nitpick comments (1)
backend/app/agents/devrel/tool_wrappers.py (1)
37-41: Minor refactor: Conditional expression for message extraction.The refactored conditional expression for extracting
latest_messageis more concise. Consider extracting this pattern into a helper function if it's used in multiple tool nodes to reduce duplication.Example helper:
def extract_latest_message(state: AgentState) -> str: """Extract latest message from state""" return ( state.messages[-1].get("content", "") if state.messages else state.context.get("original_message", "") )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/app/agents/devrel/agent.py(2 hunks)backend/app/agents/devrel/tool_wrappers.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/app/agents/devrel/tool_wrappers.py (4)
backend/app/agents/devrel/nodes/handlers/faq.py (1)
handle_faq_node(17-74)backend/app/agents/state.py (1)
AgentState(18-73)backend/app/agents/devrel/nodes/react_supervisor.py (1)
add_tool_result(163-202)backend/app/agents/devrel/nodes/handlers/onboarding.py (1)
handle_onboarding_node(6-17)
backend/app/agents/devrel/agent.py (3)
backend/app/agents/devrel/tools/faq_tool.py (1)
FAQTool(6-43)backend/app/services/embedding_service/service.py (1)
llm(55-68)backend/app/agents/devrel/tool_wrappers.py (1)
faq_handler_tool_node(18-24)
backend/app/agents/devrel/agent.py
Outdated
| ) | ||
| self.search_tool = DuckDuckGoSearchTool() | ||
| self.faq_tool = FAQTool() | ||
| self.faq_tool = FAQTool(search_tool=self.search_tool, llm=self.llm) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Remove unused FAQTool instance.
The self.faq_tool instance is created but never used. The workflow now calls handle_faq_node directly via the tool wrapper, bypassing the FAQTool class. This line (and potentially the import on line 9) can be removed.
Verify that FAQTool is not used elsewhere in the codebase:
If no other usage is found, apply this diff:
-from .tools.faq_tool import FAQTool- self.faq_tool = FAQTool(search_tool=self.search_tool, llm=self.llm)🏁 Script executed:
#!/bin/bash
# Search for usage of self.faq_tool in the DevRel agent and related files
rg -n 'self\.faq_tool' --type py -g '!__pycache__' -g '!*.pyc'Length of output: 113
🏁 Script executed:
#!/bin/bash
# Search for any usage of FAQTool instantiation or calls elsewhere
rg -n 'FAQTool\(' --type py -g '!__pycache__' -g '!*.pyc'Length of output: 113
Remove unused FAQTool import and instantiation
-from .tools.faq_tool import FAQTool
…
- self.faq_tool = FAQTool(search_tool=self.search_tool, llm=self.llm)📝 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.
| self.faq_tool = FAQTool(search_tool=self.search_tool, llm=self.llm) | |
| --- a/backend/app/agents/devrel/agent.py | |
| @@ | |
| -from .tools.faq_tool import FAQTool | |
| @@ class DevrelAgent: | |
| def __init__(self, config, llm, search_tool): | |
| # existing initialization... | |
| - self.faq_tool = FAQTool(search_tool=self.search_tool, llm=self.llm) |
🤖 Prompt for AI Agents
In backend/app/agents/devrel/agent.py around line 31, the FAQTool is imported
and instantiated but not used; remove the unused import and delete the
instantiation (self.faq_tool = FAQTool(...)) to avoid dead code, and run
tests/linting to ensure no other references remain—if other code expects
faq_tool, either wire it up properly or remove those references as well.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
backend/app/agents/devrel/prompts/faq_prompt.py (1)
15-58: Normalize punctuation for lintingThe templates include en dashes (“2–3”) and a curly apostrophe (“you’re”). Ruff flags these as ambiguous characters; replace them with ASCII hyphen-minus and straight apostrophe to keep lint clean.
-3. **Be informative yet concise (2–3 paragraphs max).** +3. **Be informative yet concise (2-3 paragraphs max).** … -3. Keep it concise (2–3 paragraphs). +3. Keep it concise (2-3 paragraphs). … -6. Avoid mentioning “search results” — write as though you’re providing expert knowledge. +6. Avoid mentioning "search results" — write as though you're providing expert knowledge.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/app/agents/devrel/agent.py(2 hunks)backend/app/agents/devrel/prompts/faq_prompt.py(1 hunks)backend/app/agents/devrel/tool_wrappers.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/app/agents/devrel/tool_wrappers.py (3)
backend/app/agents/devrel/nodes/handlers/faq.py (1)
handle_faq_node(17-74)backend/app/agents/state.py (1)
AgentState(18-73)backend/app/agents/devrel/nodes/react_supervisor.py (1)
add_tool_result(163-202)
backend/app/agents/devrel/agent.py (3)
backend/app/agents/devrel/tools/faq_tool.py (1)
FAQTool(6-43)backend/app/services/embedding_service/service.py (1)
llm(55-68)backend/app/agents/devrel/tool_wrappers.py (1)
faq_handler_tool_node(18-24)
🪛 Ruff (0.13.3)
backend/app/agents/devrel/prompts/faq_prompt.py
15-15: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF001)
54-54: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF001)
57-57: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
| ### Instructions: | ||
| 1. **Answer the user's question directly and conversationally.** | ||
| 2. **Focus on the most relevant and recent information.** | ||
| 3. **Be informative yet concise (2–3 paragraphs max).** | ||
| 4. **If the search results don't fully answer the question, acknowledge that clearly.** | ||
| 5. **Maintain a knowledgeable and professional tone** as if you are part of {org_name}. | ||
| 6. **Do not mention "search results" or sources** — respond as if you know the information first-hand. | ||
| --- | ||
| ### Your Response: | ||
| """ | ||
| SYNTHESIS_PROMPT = """ | ||
| You are an AI assistant representing **{org_name}**. | ||
| A user asked: "{message}" | ||
| Below are search results gathered from official sources about {org_name}. | ||
| Search Results: | ||
| {results_context} | ||
| --- | ||
| ### Step 1: Refine User Query | ||
| Refine the user's question to make it suitable for an internal FAQ or search engine query. | ||
| **Organization:** {org_name} | ||
| **Original User Question:** {message} | ||
| --- | ||
| ### Step 2: Synthesize the Information | ||
| Based on the refined query and search context, generate a clear, accurate, and conversational answer. | ||
| **Search Results Context:** | ||
| {results_context} | ||
| --- | ||
| ### Step 3: Write the Final Answer | ||
| 1. Respond directly and conversationally. | ||
| 2. Highlight the most relevant and updated facts. | ||
| 3. Keep it concise (2–3 paragraphs). | ||
| 4. Acknowledge any missing information if applicable. | ||
| 5. Sound professional and informed about {org_name}. | ||
| 6. Avoid mentioning “search results” — write as though you’re providing expert knowledge. |
There was a problem hiding this comment.
Restore source attribution in the FAQ prompts
The dynamic FAQ feature must return source links, but both prompts explicitly forbid mentioning sources. That contradicts the product requirement (“Provide source links for verification”) and will suppress citations at runtime. Update the instructions/response sections so the model is told to include the extracted sources (e.g., “Append a bulleted ‘Sources’ section with the URLs you used”).
🧰 Tools
🪛 Ruff (0.13.3)
15-15: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF001)
54-54: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF001)
57-57: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
backend/app/agents/devrel/prompts/faq_prompt.py (1)
24-62: Consider consolidating duplicative prompt logic.Both
REFINEMENT_PROMPTandSYNTHESIS_PROMPTcontain similar instructions and formatting. The main difference is thatSYNTHESIS_PROMPTadds a three-step structure. Consider whether both are necessary or if one can be parameterized to serve both purposes.backend/app/agents/devrel/tools/faq_tool.py (4)
98-107: Improve exception logging for better debugging.Use
logging.exception()instead oflogging.error()to automatically include the stack trace, which aids in debugging.Apply this diff:
except Exception as e: - logger.error(f"[FAQ_TOOL] Failed to handle FAQ: {e}") + logger.exception(f"[FAQ_TOOL] Failed to handle FAQ: {e}") return {
115-122: Consider logging URL parsing failures for observability.While silently continuing on invalid URLs is reasonable, logging failures at debug level could help diagnose configuration issues.
try: parsed = urlparse(u) domain = parsed.netloc or parsed.path if domain: domains.append(domain) except Exception: + logger.debug(f"[FAQ_TOOL] Failed to parse handle URL: {u}") continue
146-148: Improve exception logging in search and pipeline error handlers.Use
logging.exception()to include stack traces for better debugging of search failures and pipeline errors.Apply this diff:
except Exception as err: - logger.error(f"[FAQ_TOOL] Search failed: {err}") + logger.exception(f"[FAQ_TOOL] Search failed: {err}") return self._generate_fallback_response(message)except Exception as e: - logger.error(f"[FAQ_TOOL] Error in dynamic FAQ process: {e}") + logger.exception(f"[FAQ_TOOL] Error in dynamic FAQ process: {e}") return self._generate_fallback_response(message)Also applies to: 162-164
167-175: Consider adding timeout to query refinement LLM call for consistency.The synthesis step (line 198) uses
asyncio.wait_for()withFAQ_LLM_TIMEOUT, but this refinement call does not. For consistency and resilience, consider adding a timeout here as well.async def _refine_faq_query(self, message: str) -> str: """Refine user's question into an optimized search query.""" refinement_prompt = load_prompt("refinement_prompt").format( org_name=ORG_NAME, message=message ) - response = await self.llm.ainvoke([HumanMessage(content=refinement_prompt)]) + response = await asyncio.wait_for( + self.llm.ainvoke([HumanMessage(content=refinement_prompt)]), + timeout=FAQ_LLM_TIMEOUT + ) refined_query = response.content.strip() logger.info(f"[FAQ_TOOL] Refined query: {refined_query}") return refined_query
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/app/agents/devrel/prompts/faq_prompt.py(1 hunks)backend/app/agents/devrel/tools/faq_tool.py(1 hunks)backend/app/database/weaviate/scripts/create_schemas.py(0 hunks)backend/app/database/weaviate/scripts/populate_db.py(1 hunks)tests/test_embedding_service.py(0 hunks)
💤 Files with no reviewable changes (2)
- tests/test_embedding_service.py
- backend/app/database/weaviate/scripts/create_schemas.py
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/agents/devrel/tools/faq_tool.py (2)
backend/app/agents/state.py (1)
AgentState(18-73)backend/app/agents/devrel/nodes/handlers/faq.py (6)
_generate_fallback_response(216-228)_dynamic_faq_process(77-126)_refine_faq_query(129-154)_synthesize_faq_response(157-197)_format_faq_response(200-213)handle_faq_node(17-74)
🪛 Ruff (0.13.3)
backend/app/agents/devrel/prompts/faq_prompt.py
15-15: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF001)
54-54: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF001)
57-57: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
backend/app/agents/devrel/tools/faq_tool.py
39-41: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
39-41: Avoid specifying long messages outside the exception class
(TRY003)
89-96: Consider moving this statement to an else block
(TRY300)
98-98: Do not catch blind exception: Exception
(BLE001)
99-99: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
121-122: try-except-continue detected, consider logging the exception
(S112)
121-121: Do not catch blind exception: Exception
(BLE001)
146-146: Do not catch blind exception: Exception
(BLE001)
147-147: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
154-154: Comment contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF003)
162-162: Do not catch blind exception: Exception
(BLE001)
163-163: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
224-224: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
224-224: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
227-227: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
🔇 Additional comments (4)
backend/app/database/weaviate/scripts/populate_db.py (1)
305-305: Confirm all references to running populate_db.py are updated. No direct invocations or imports were found; manually verify CI/CD workflows, Makefiles, shell scripts, shebang calls, and documentation for any remaining calls.backend/app/agents/devrel/tools/faq_tool.py (3)
11-18: LGTM! Clean configuration integration.The organization settings and timeout configurations are properly initialized with sensible defaults.
198-201: LGTM! Proper timeout handling on LLM synthesis.The use of
asyncio.wait_for()withFAQ_LLM_TIMEOUTensures the synthesis step doesn't hang indefinitely.
233-239: LGTM! Clean compatibility wrapper for migration.The wrapper provides a smooth migration path for existing code while encouraging new code to use
FAQTool.get_response()directly.
| ### Instructions: | ||
| 1. **Answer the user's question directly and conversationally.** | ||
| 2. **Focus on the most relevant and recent information.** | ||
| 3. **Be informative yet concise (2–3 paragraphs max).** |
There was a problem hiding this comment.
Fix ambiguous characters flagged by static analysis.
The prompts contain ambiguous Unicode characters that should be replaced with standard ASCII equivalents for consistency:
- EN DASH (–) should be HYPHEN-MINUS (-)
- RIGHT SINGLE QUOTATION MARK (') should be APOSTROPHE (')
Apply this diff to fix the ambiguous characters:
-3. **Be informative yet concise (2–3 paragraphs max).**
+3. **Be informative yet concise (2-3 paragraphs max).**-3. Keep it concise (2–3 paragraphs).
+3. Keep it concise (2-3 paragraphs). -6. Avoid mentioning "search results" — write as though you're providing expert knowledge.
+6. Avoid mentioning "search results" - write as though you're providing expert knowledge.Also applies to: 54-54, 57-57
🧰 Tools
🪛 Ruff (0.13.3)
15-15: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF001)
🤖 Prompt for AI Agents
In backend/app/agents/devrel/prompts/faq_prompt.py around lines 15, 54 and 57,
replace ambiguous Unicode punctuation with ASCII equivalents: change EN DASH (–)
to HYPHEN-MINUS (-) and RIGHT SINGLE QUOTATION MARK (’) to APOSTROPHE (')
wherever they appear in the prompt strings; update the text so punctuation is
standard ASCII throughout and save the file.
| FAQ_LLM_TIMEOUT = getattr(app_settings, "faq_llm_timeout", 15.0) | ||
|
|
||
| # Path to FAQ prompts | ||
| FAQ_PROMPTS_PATH = Path(__file__).parent / "prompt" / "faq_prompt.py" |
There was a problem hiding this comment.
Incorrect directory name in path construction.
The path uses "prompt" but the actual directory is "prompts" (as evidenced by the import on line 36). This will cause file lookups to fail.
Apply this diff:
-FAQ_PROMPTS_PATH = Path(__file__).parent / "prompt" / "faq_prompt.py"
+FAQ_PROMPTS_PATH = Path(__file__).parent.parent / "prompts"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/app/agents/devrel/tools/faq_tool.py around line 21, the Path is
constructed with the wrong directory name ("prompt"); change it to the correct
directory "prompts" so FAQ_PROMPTS_PATH = Path(__file__).parent / "prompts" /
"faq_prompt.py" (update the string only) to match the actual import and avoid
file lookup failures.
| # 2. Fallback: Load from Python module | ||
| try: | ||
| from app.agents.devrel.prompts.faq_prompt import REFINEMENT_PROMPT | ||
| return getattr(REFINEMENT_PROMPT, name) | ||
| except (ImportError, AttributeError): | ||
| raise FileNotFoundError( | ||
| f"Prompt '{name}' not found in {prompt_path} or faq_prompt.py" | ||
| ) |
There was a problem hiding this comment.
Critical logic error in prompt loading fallback.
Line 37 attempts getattr(REFINEMENT_PROMPT, name) where REFINEMENT_PROMPT is a string constant, not a module. This will always fail with AttributeError.
Apply this diff to correctly load from the module:
# 2. Fallback: Load from Python module
try:
- from app.agents.devrel.prompts.faq_prompt import REFINEMENT_PROMPT
- return getattr(REFINEMENT_PROMPT, name)
+ from app.agents.devrel.prompts import faq_prompt
+ return getattr(faq_prompt, name.upper())
except (ImportError, AttributeError):
raise FileNotFoundError(
f"Prompt '{name}' not found in {prompt_path} or faq_prompt.py"
- )
+ ) from NoneNote: Also added from None to properly chain exceptions as suggested by static analysis.
📝 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.
| # 2. Fallback: Load from Python module | |
| try: | |
| from app.agents.devrel.prompts.faq_prompt import REFINEMENT_PROMPT | |
| return getattr(REFINEMENT_PROMPT, name) | |
| except (ImportError, AttributeError): | |
| raise FileNotFoundError( | |
| f"Prompt '{name}' not found in {prompt_path} or faq_prompt.py" | |
| ) | |
| # 2. Fallback: Load from Python module | |
| try: | |
| from app.agents.devrel.prompts import faq_prompt | |
| return getattr(faq_prompt, name.upper()) | |
| except (ImportError, AttributeError): | |
| raise FileNotFoundError( | |
| f"Prompt '{name}' not found in {prompt_path} or faq_prompt.py" | |
| ) from None |
🧰 Tools
🪛 Ruff (0.13.3)
39-41: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
39-41: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In backend/app/agents/devrel/tools/faq_tool.py around lines 34 to 41, the
fallback attempts getattr(REFINEMENT_PROMPT, name) but REFINEMENT_PROMPT is a
string constant so getattr will always raise AttributeError; instead import the
module object (e.g., from app.agents.devrel.prompts import faq_prompt or use
importlib.import_module("app.agents.devrel.prompts.faq_prompt")) and call
getattr(faq_prompt, name) to retrieve the prompt, and when re-raising wrap the
original exception with "from None" removed and instead use "raise
FileNotFoundError(...) from e" so the exception chain is preserved.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/app/agents/devrel/tools/faq_tool.py(1 hunks)backend/app/database/weaviate/scripts/create_schemas.py(0 hunks)backend/app/database/weaviate/scripts/populate_db.py(1 hunks)tests/test_embedding_service.py(0 hunks)
💤 Files with no reviewable changes (2)
- tests/test_embedding_service.py
- backend/app/database/weaviate/scripts/create_schemas.py
✅ Files skipped from review due to trivial changes (1)
- backend/app/database/weaviate/scripts/populate_db.py
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/agents/devrel/tools/faq_tool.py (2)
backend/app/agents/state.py (1)
AgentState(18-73)backend/app/agents/devrel/nodes/handlers/faq.py (6)
_generate_fallback_response(216-228)_dynamic_faq_process(77-126)_refine_faq_query(129-154)_synthesize_faq_response(157-197)_format_faq_response(200-213)handle_faq_node(17-74)
🪛 Ruff (0.13.3)
backend/app/agents/devrel/tools/faq_tool.py
39-41: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
39-41: Avoid specifying long messages outside the exception class
(TRY003)
89-96: Consider moving this statement to an else block
(TRY300)
98-98: Do not catch blind exception: Exception
(BLE001)
99-99: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
121-122: try-except-continue detected, consider logging the exception
(S112)
121-121: Do not catch blind exception: Exception
(BLE001)
146-146: Do not catch blind exception: Exception
(BLE001)
147-147: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
154-154: Comment contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF003)
162-162: Do not catch blind exception: Exception
(BLE001)
163-163: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
224-224: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
224-224: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
227-227: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
| FAQ_PROMPTS_PATH = Path(__file__).parent / "prompt" / "faq_prompt.py" | ||
|
|
||
| def load_prompt(name: str) -> str: | ||
| """ | ||
| Load a prompt from a .txt file or fallback to faq_prompt.py. | ||
| """ | ||
| prompt_path = FAQ_PROMPTS_PATH / f"{name}.txt" | ||
|
|
||
| # 1. Try to load from text file | ||
| if prompt_path.exists(): | ||
| with open(prompt_path, "r", encoding="utf-8") as f: | ||
| return f.read() | ||
|
|
||
| # 2. Fallback: Load from Python module | ||
| try: | ||
| from app.agents.devrel.prompts.faq_prompt import REFINEMENT_PROMPT | ||
| return getattr(REFINEMENT_PROMPT, name) | ||
| except (ImportError, AttributeError): | ||
| raise FileNotFoundError( | ||
| f"Prompt '{name}' not found in {prompt_path} or faq_prompt.py" | ||
| ) |
There was a problem hiding this comment.
Fix prompt loading so the dynamic FAQ pipeline can run
load_prompt currently can’t return anything: FAQ_PROMPTS_PATH points to the faq_prompt.py file, so prompt_path.exists() is always false, and the fallback tries getattr on the imported REFINEMENT_PROMPT string, immediately raising AttributeError. The first call to load_prompt("refinement_prompt") therefore throws a FileNotFoundError, causing _dynamic_faq_process to short‑circuit into the fallback response for every FAQ request. The new dynamic handler never actually searches or synthesizes results.
Please point the loader at the prompts directory and fetch the string constants from the module itself. One way:
-FAQ_PROMPTS_PATH = Path(__file__).parent / "prompt" / "faq_prompt.py"
+FAQ_PROMPTS_DIR = Path(__file__).parent.parent / "prompts"
def load_prompt(name: str) -> str:
@@
- prompt_path = FAQ_PROMPTS_PATH / f"{name}.txt"
+ prompt_path = FAQ_PROMPTS_DIR / f"{name}.txt"
@@
- from app.agents.devrel.prompts.faq_prompt import REFINEMENT_PROMPT
- return getattr(REFINEMENT_PROMPT, name)
- except (ImportError, AttributeError):
+ from app.agents.devrel.prompts import faq_prompt as prompt_module
+ attr_name = name.upper()
+ if hasattr(prompt_module, attr_name):
+ return getattr(prompt_module, attr_name)
+ except ImportError as err:
+ raise FileNotFoundError(
+ f"Prompt '{name}' not found in {prompt_path} or faq_prompt.py"
+ ) from err
+ raise FileNotFoundError(
+ f"Prompt '{name}' not found in {prompt_path} or faq_prompt.py"
+ )That restoration will let the refinement/synthesis prompts resolve and the dynamic FAQ flow execute as designed.
📝 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.
| FAQ_PROMPTS_PATH = Path(__file__).parent / "prompt" / "faq_prompt.py" | |
| def load_prompt(name: str) -> str: | |
| """ | |
| Load a prompt from a .txt file or fallback to faq_prompt.py. | |
| """ | |
| prompt_path = FAQ_PROMPTS_PATH / f"{name}.txt" | |
| # 1. Try to load from text file | |
| if prompt_path.exists(): | |
| with open(prompt_path, "r", encoding="utf-8") as f: | |
| return f.read() | |
| # 2. Fallback: Load from Python module | |
| try: | |
| from app.agents.devrel.prompts.faq_prompt import REFINEMENT_PROMPT | |
| return getattr(REFINEMENT_PROMPT, name) | |
| except (ImportError, AttributeError): | |
| raise FileNotFoundError( | |
| f"Prompt '{name}' not found in {prompt_path} or faq_prompt.py" | |
| ) | |
| FAQ_PROMPTS_DIR = Path(__file__).parent.parent / "prompts" | |
| def load_prompt(name: str) -> str: | |
| """ | |
| Load a prompt from a .txt file or fallback to faq_prompt.py. | |
| """ | |
| prompt_path = FAQ_PROMPTS_DIR / f"{name}.txt" | |
| # 1. Try to load from text file | |
| if prompt_path.exists(): | |
| with open(prompt_path, "r", encoding="utf-8") as f: | |
| return f.read() | |
| # 2. Fallback: Load from Python module | |
| - try: | |
| - from app.agents.devrel.prompts.faq_prompt import REFINEMENT_PROMPT | |
| - return getattr(REFINEMENT_PROMPT, name) | |
| - except (ImportError, AttributeError): | |
| - raise FileNotFoundError( | |
| - f"Prompt '{name}' not found in {prompt_path} or faq_prompt.py" | |
| try: | |
| from app.agents.devrel.prompts import faq_prompt as prompt_module | |
| attr_name = name.upper() | |
| if hasattr(prompt_module, attr_name): | |
| return getattr(prompt_module, attr_name) | |
| except ImportError as err: | |
| raise FileNotFoundError( | |
| f"Prompt '{name}' not found in {prompt_path} or faq_prompt.py" | |
| ) from err | |
| raise FileNotFoundError( | |
| f"Prompt '{name}' not found in {prompt_path} or faq_prompt.py" | |
| ) |
🧰 Tools
🪛 Ruff (0.13.3)
39-41: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
39-41: Avoid specifying long messages outside the exception class
(TRY003)
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
backend/app/agents/devrel/prompts/faq_prompt.py (1)
1-62: Consider consolidating the two similar prompts.Both
REFINEMENT_PROMPTandSYNTHESIS_PROMPTcontain very similar instructions (conversational tone, 2-3 paragraphs, don't mention sources, acknowledge gaps). The main difference is thatSYNTHESIS_PROMPTincludes explicit step headers.Consider whether both are necessary or if a single, well-structured prompt could serve both purposes. This would reduce maintenance overhead and prevent instruction drift between the two.
backend/app/agents/devrel/tools/faq_tool.py (2)
99-99: Use logging.exception for better error diagnostics.When logging errors in exception handlers, use
logging.exception()instead oflogging.error()to automatically include the stack trace.Apply this diff:
except Exception as e: - logger.error(f"[FAQ_TOOL] Failed to handle FAQ: {e}") + logger.exception(f"[FAQ_TOOL] Failed to handle FAQ: {e}")Same issue on lines 147 and 163.
39-41: Chain exception properly withfromclause.When raising exceptions in an
exceptblock, useraise ... from errorraise ... from Noneto properly chain exceptions and preserve the original traceback.Apply this diff:
except (ImportError, AttributeError): - raise FileNotFoundError( + raise FileNotFoundError( f"Prompt '{name}' not found in {prompt_path} or faq_prompt.py" - ) + ) from None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/app/agents/devrel/prompts/faq_prompt.py(1 hunks)backend/app/agents/devrel/tools/faq_tool.py(1 hunks)backend/app/database/weaviate/scripts/create_schemas.py(0 hunks)backend/app/database/weaviate/scripts/populate_db.py(1 hunks)tests/test_embedding_service.py(0 hunks)
💤 Files with no reviewable changes (2)
- backend/app/database/weaviate/scripts/create_schemas.py
- tests/test_embedding_service.py
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/agents/devrel/tools/faq_tool.py (3)
backend/app/agents/state.py (1)
AgentState(18-73)backend/app/services/embedding_service/service.py (1)
llm(55-68)backend/app/agents/devrel/nodes/handlers/faq.py (6)
_generate_fallback_response(216-228)_dynamic_faq_process(77-126)_refine_faq_query(129-154)_synthesize_faq_response(157-197)_format_faq_response(200-213)handle_faq_node(17-74)
🪛 Ruff (0.13.3)
backend/app/agents/devrel/tools/faq_tool.py
39-41: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
39-41: Avoid specifying long messages outside the exception class
(TRY003)
89-96: Consider moving this statement to an else block
(TRY300)
98-98: Do not catch blind exception: Exception
(BLE001)
99-99: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
121-122: try-except-continue detected, consider logging the exception
(S112)
121-121: Do not catch blind exception: Exception
(BLE001)
146-146: Do not catch blind exception: Exception
(BLE001)
147-147: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
154-154: Comment contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF003)
162-162: Do not catch blind exception: Exception
(BLE001)
163-163: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
224-224: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
224-224: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
227-227: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
backend/app/agents/devrel/prompts/faq_prompt.py
15-15: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF001)
54-54: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF001)
57-57: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
🔇 Additional comments (1)
backend/app/database/weaviate/scripts/populate_db.py (1)
305-305: Approve removal of__main__guard: no internal references found
The deletion cleanly disables direct execution; no code, documentation, or CI workflows in the repo referencepopulate_db.py.
| ### Instructions: | ||
| 1. **Answer the user's question directly and conversationally.** | ||
| 2. **Focus on the most relevant and recent information.** | ||
| 3. **Be informative yet concise (2–3 paragraphs max).** |
There was a problem hiding this comment.
Replace ambiguous EN DASH with standard hyphen.
The string contains an EN DASH (–) which may cause text processing issues. Replace with a standard HYPHEN-MINUS (-).
Apply this diff:
-3. **Be informative yet concise (2–3 paragraphs max).**
+3. **Be informative yet concise (2-3 paragraphs max).**📝 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.
| 3. **Be informative yet concise (2–3 paragraphs max).** | |
| 3. **Be informative yet concise (2-3 paragraphs max).** |
🧰 Tools
🪛 Ruff (0.13.3)
15-15: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF001)
🤖 Prompt for AI Agents
In backend/app/agents/devrel/prompts/faq_prompt.py around line 15, the text
"2–3" uses an EN DASH (U+2013); replace that character with a standard
hyphen-minus (U+002D) so it reads "2-3", updating the file encoding/commit as
needed.
| ### Step 3: Write the Final Answer | ||
| 1. Respond directly and conversationally. | ||
| 2. Highlight the most relevant and updated facts. | ||
| 3. Keep it concise (2–3 paragraphs). |
There was a problem hiding this comment.
Replace ambiguous EN DASH with standard hyphen.
The string contains an EN DASH (–) which may cause text processing issues. Replace with a standard HYPHEN-MINUS (-).
Apply this diff:
-3. Keep it concise (2–3 paragraphs).
+3. Keep it concise (2-3 paragraphs).📝 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.
| 3. Keep it concise (2–3 paragraphs). | |
| 3. Keep it concise (2-3 paragraphs). |
🧰 Tools
🪛 Ruff (0.13.3)
54-54: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF001)
🤖 Prompt for AI Agents
In backend/app/agents/devrel/prompts/faq_prompt.py around line 54, replace the
ambiguous EN DASH (U+2013, “–”) used in the string "2–3 paragraphs" with a
standard hyphen-minus ("-") so it reads "2-3 paragraphs"; update the source text
accordingly (search and replace the character) and save the file with UTF-8
encoding to avoid text-processing issues.
| 3. Keep it concise (2–3 paragraphs). | ||
| 4. Acknowledge any missing information if applicable. | ||
| 5. Sound professional and informed about {org_name}. | ||
| 6. Avoid mentioning “search results” — write as though you’re providing expert knowledge. |
There was a problem hiding this comment.
Replace ambiguous RIGHT SINGLE QUOTATION MARK with standard apostrophe.
The string contains a RIGHT SINGLE QUOTATION MARK (') which may cause text processing issues. Replace with a standard apostrophe (').
Apply this diff:
-6. Avoid mentioning "search results" — write as though you're providing expert knowledge.
+6. Avoid mentioning "search results" — write as though you're providing expert knowledge.📝 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.
| 6. Avoid mentioning “search results” — write as though you’re providing expert knowledge. | |
| 6. Avoid mentioning “search results” — write as though you're providing expert knowledge. |
🧰 Tools
🪛 Ruff (0.13.3)
57-57: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
🤖 Prompt for AI Agents
In backend/app/agents/devrel/prompts/faq_prompt.py around line 57, there is a
RIGHT SINGLE QUOTATION MARK character used in the string; replace that character
with the standard ASCII apostrophe (') so the string uses a normal apostrophe,
ensuring file encoding remains UTF-8 and updating any affected string literals
accordingly.
| FAQ_LLM_TIMEOUT = getattr(app_settings, "faq_llm_timeout", 15.0) | ||
|
|
||
| # Path to FAQ prompts | ||
| FAQ_PROMPTS_PATH = Path(__file__).parent / "prompt" / "faq_prompt.py" |
There was a problem hiding this comment.
Critical: FAQ_PROMPTS_PATH points to a Python file, not a directory.
FAQ_PROMPTS_PATH is constructed as Path(__file__).parent / "prompt" / "faq_prompt.py", which results in a path ending in .py (a Python file). This breaks the path construction on line 27 where you append f"{name}.txt", creating invalid paths like .../prompt/faq_prompt.py/refinement_prompt.txt.
Fix by pointing to the prompts directory instead:
-FAQ_PROMPTS_PATH = Path(__file__).parent / "prompt" / "faq_prompt.py"
+FAQ_PROMPTS_PATH = Path(__file__).parent.parent / "prompts"📝 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.
| FAQ_PROMPTS_PATH = Path(__file__).parent / "prompt" / "faq_prompt.py" | |
| FAQ_PROMPTS_PATH = Path(__file__).parent.parent / "prompts" |
🤖 Prompt for AI Agents
In backend/app/agents/devrel/tools/faq_tool.py around line 21, FAQ_PROMPTS_PATH
is set to the file path Path(__file__).parent / "prompt" / "faq_prompt.py" which
yields a .py file and breaks later when appending f"{name}.txt"; change it to
point to the prompts directory instead (e.g., Path(__file__).parent / "prompt")
so subsequent path joins produce valid files like
.../prompt/refinement_prompt.txt; update the constant accordingly and ensure any
usages that append filenames remain unchanged.
| from app.agents.devrel.prompts.faq_prompt import REFINEMENT_PROMPT | ||
| return getattr(REFINEMENT_PROMPT, name) |
There was a problem hiding this comment.
Critical: getattr called on string constant instead of module.
Line 36 imports REFINEMENT_PROMPT (a string constant), then line 37 attempts getattr(REFINEMENT_PROMPT, name). Since REFINEMENT_PROMPT is a string, not a module, this will always raise AttributeError.
Fix by importing the module and accessing its attributes:
# 2. Fallback: Load from Python module
try:
- from app.agents.devrel.prompts.faq_prompt import REFINEMENT_PROMPT
- return getattr(REFINEMENT_PROMPT, name)
+ from app.agents.devrel.prompts import faq_prompt
+ # Convert snake_case name to UPPER_CASE constant name
+ constant_name = name.upper()
+ return getattr(faq_prompt, constant_name)
except (ImportError, AttributeError):📝 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.
| from app.agents.devrel.prompts.faq_prompt import REFINEMENT_PROMPT | |
| return getattr(REFINEMENT_PROMPT, name) | |
| # 2. Fallback: Load from Python module | |
| try: | |
| - from app.agents.devrel.prompts.faq_prompt import REFINEMENT_PROMPT | |
| from app.agents.devrel.prompts import faq_prompt | |
| # Convert snake_case name to UPPER_CASE constant name | |
| constant_name = name.upper() | |
| return getattr(faq_prompt, constant_name) | |
| except (ImportError, AttributeError): | |
| # handle missing module or constant | |
| ... |
🤖 Prompt for AI Agents
In backend/app/agents/devrel/tools/faq_tool.py around lines 36-37, the code
imports REFINEMENT_PROMPT (a string constant) then calls
getattr(REFINEMENT_PROMPT, name), which will raise AttributeError; fix it by
importing the module object instead of the string constant (for example import
or from app.agents.devrel.prompts import faq_prompt or import
app.agents.devrel.prompts.faq_prompt as faq_prompt) and then call
getattr(faq_prompt, name); optionally keep a clear fallback or raise a
descriptive error if the attribute isn’t found.
| class FAQTool: | ||
| """FAQ handling tool""" | ||
|
|
||
| # TODO: Add FAQ responses from a database to refer organization's FAQ and Repo's FAQ | ||
|
|
||
| def __init__(self): | ||
| self.faq_responses = { | ||
| "what is devr.ai": "Devr.AI is an AI-powered Developer Relations assistant that helps open-source communities by automating engagement, issue tracking, and providing intelligent support to developers.", | ||
| "how do i contribute": "You can contribute by visiting our GitHub repository, checking open issues, and submitting pull requests. We welcome all types of contributions including code, documentation, and bug reports.", | ||
| "what platforms does devr.ai support": "Devr.AI integrates with Discord, Slack, GitHub, and can be extended to other platforms. We use these integrations to provide seamless developer support across multiple channels.", | ||
| "who maintains devr.ai": "Devr.AI is maintained by an open-source community of developers passionate about improving developer relations and community engagement.", | ||
| "how do i report a bug": "You can report a bug by opening an issue on our GitHub repository. Please include detailed information about the bug, steps to reproduce it, and your environment.", | ||
| "how to get started": "To get started with Devr.AI: 1) Check our documentation, 2) Join our Discord community, 3) Explore the GitHub repository, 4) Try contributing to open issues.", | ||
| "what is langgraph": "LangGraph is a framework for building stateful, multi-actor applications with large language models. We use it to create intelligent agent workflows for our DevRel automation." | ||
| } | ||
|
|
||
| async def get_response(self, question: str) -> Optional[str]: | ||
| """Get FAQ response for a question""" | ||
| question_lower = question.lower().strip() | ||
|
|
||
| # Direct match | ||
| if question_lower in self.faq_responses: | ||
| return self.faq_responses[question_lower] | ||
|
|
||
| # Fuzzy matching | ||
| for faq_key, response in self.faq_responses.items(): | ||
| if self._is_similar_question(question_lower, faq_key): | ||
| return response | ||
|
|
||
| return None | ||
|
|
||
| def _is_similar_question(self, question: str, faq_key: str) -> bool: | ||
| """Check if question is similar to FAQ key""" | ||
| # Simple keyword matching - in production, use better similarity | ||
| question_words = set(question.split()) | ||
| faq_words = set(faq_key.split()) | ||
|
|
||
| common_words = question_words.intersection(faq_words) | ||
| return len(common_words) >= 2 # At least 2 common words | ||
| """ | ||
| Dynamic FAQ handling tool integrating search and LLM synthesis. | ||
| Handles FAQ queries by refining search terms, fetching relevant data, | ||
| and synthesizing coherent responses with source citations. | ||
| """ | ||
|
|
||
| def __init__(self, search_tool: Any, llm: Any): | ||
| self.search_tool = search_tool | ||
| self.llm = llm | ||
|
|
||
| async def get_response(self, state: AgentState) -> Optional[Dict[str, Any]]: | ||
| """ | ||
| Fetch and synthesize a dynamic FAQ response using web search and LLM. | ||
| """ | ||
| logger.info(f"Handling FAQ for session {state.session_id}") | ||
|
|
||
| # Extract latest user message | ||
| latest_message = ( | ||
| state.messages[-1].get("content", "") | ||
| if state.messages | ||
| else state.context.get("original_message", "") | ||
| ) | ||
|
|
||
| if not latest_message: | ||
| logger.warning("[FAQ_TOOL] Empty user message — returning fallback") | ||
| return { | ||
| "task_result": { | ||
| "type": "faq", | ||
| "response": self._generate_fallback_response(latest_message), | ||
| "source": "dynamic_web_search", | ||
| }, | ||
| "current_task": "faq_handled", | ||
| } | ||
|
|
||
| try: | ||
| # Build site filters from official handles | ||
| site_filters = self._build_site_filters() | ||
| logger.info(f"[FAQ_TOOL] Using site filters: {site_filters or '(none)'}") | ||
|
|
||
| # Process FAQ request | ||
| faq_response = await self._dynamic_faq_process( | ||
| latest_message, site_filters=site_filters | ||
| ) | ||
|
|
||
| return { | ||
| "task_result": { | ||
| "type": "faq", | ||
| "response": faq_response, | ||
| "source": "dynamic_web_search", | ||
| }, | ||
| "current_task": "faq_handled", | ||
| } | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"[FAQ_TOOL] Failed to handle FAQ: {e}") | ||
| return { | ||
| "task_result": { | ||
| "type": "faq", | ||
| "response": f"Sorry, something went wrong while handling your FAQ request: {e}", | ||
| "source": "error", | ||
| }, | ||
| "current_task": "faq_failed", | ||
| } | ||
|
|
||
|
|
||
| def _build_site_filters(self) -> str: | ||
| """Construct search site filters from official handles.""" | ||
| from urllib.parse import urlparse | ||
|
|
||
| domains = [] | ||
| for u in OFFICIAL_HANDLES: | ||
| try: | ||
| parsed = urlparse(u) | ||
| domain = parsed.netloc or parsed.path | ||
| if domain: | ||
| domains.append(domain) | ||
| except Exception: | ||
| continue | ||
|
|
||
| return " OR ".join([f"site:{d}" for d in domains]) if domains else "" | ||
|
|
||
|
|
||
| async def _dynamic_faq_process(self, message: str, site_filters: str = "") -> str: | ||
| """ | ||
| Pipeline: refine → search → synthesize → format | ||
| """ | ||
| try: | ||
| # Step 1: Refine the query | ||
| logger.debug(f"[FAQ_TOOL] Refining query for org '{ORG_NAME}'") | ||
| refined_query = await self._refine_faq_query(message) | ||
|
|
||
| if site_filters: | ||
| refined_query = f"({refined_query}) AND ({site_filters})" | ||
| logger.debug(f"[FAQ_TOOL] Final query: {refined_query}") | ||
|
|
||
| # Step 2: Search | ||
| logger.info(f"[FAQ_TOOL] Searching for: {refined_query}") | ||
| try: | ||
| search_results = await asyncio.wait_for( | ||
| self.search_tool.search(refined_query), timeout=FAQ_SEARCH_TIMEOUT | ||
| ) | ||
| except Exception as err: | ||
| logger.error(f"[FAQ_TOOL] Search failed: {err}") | ||
| return self._generate_fallback_response(message) | ||
|
|
||
| if not search_results: | ||
| logger.warning(f"[FAQ_TOOL] No results for: {refined_query}") | ||
| return self._generate_fallback_response(message) | ||
|
|
||
| # Step 3–4: Synthesize response | ||
| logger.info("[FAQ_TOOL] Synthesizing FAQ response") | ||
| synthesized = await self._synthesize_faq_response(message, search_results) | ||
|
|
||
| # Step 5: Format final response | ||
| logger.info("[FAQ_TOOL] Formatting response with citations") | ||
| return self._format_faq_response(synthesized, search_results) | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"[FAQ_TOOL] Error in dynamic FAQ process: {e}") | ||
| return self._generate_fallback_response(message) | ||
|
|
||
|
|
||
| async def _refine_faq_query(self, message: str) -> str: | ||
| """Refine user's question into an optimized search query.""" | ||
| refinement_prompt = load_prompt("refinement_prompt").format( | ||
| org_name=ORG_NAME, message=message | ||
| ) | ||
| response = await self.llm.ainvoke([HumanMessage(content=refinement_prompt)]) | ||
| refined_query = response.content.strip() | ||
| logger.info(f"[FAQ_TOOL] Refined query: {refined_query}") | ||
| return refined_query | ||
|
|
||
|
|
||
| async def _synthesize_faq_response( | ||
| self, message: str, search_results: List[Dict[str, Any]] | ||
| ) -> str: | ||
| """Generate a synthesized answer from search results.""" | ||
| # Build context (top 5 results) | ||
| results_context = "" | ||
| for i, result in enumerate(search_results[:5]): | ||
| title = result.get("title", "N/A") | ||
| content = result.get("content", "N/A") | ||
| if isinstance(content, str) and len(content) > 500: | ||
| content = content[:500] + "..." | ||
| url = result.get("url", "N/A") | ||
| results_context += f"\nResult {i+1}:\nTitle: {title}\nContent: {content}\nURL: {url}\n" | ||
|
|
||
| # Build synthesis prompt | ||
| synthesis_prompt = load_prompt("synthesis_prompt").format( | ||
| org_name=ORG_NAME, message=message, results_context=results_context | ||
| ) | ||
|
|
||
| # LLM synthesis | ||
| response = await asyncio.wait_for( | ||
| self.llm.ainvoke([HumanMessage(content=synthesis_prompt)]), | ||
| timeout=FAQ_LLM_TIMEOUT, | ||
| ) | ||
|
|
||
| synthesized_answer = response.content.strip() | ||
| logger.debug(f"[FAQ_TOOL] Synthesized response: {synthesized_answer[:100]}...") | ||
| return synthesized_answer | ||
|
|
||
|
|
||
| def _format_faq_response( | ||
| self, synthesized_answer: str, search_results: List[Dict[str, Any]] | ||
| ) -> str: | ||
| """Append top sources to the synthesized answer.""" | ||
| formatted = synthesized_answer | ||
| if search_results: | ||
| formatted += "\n\n**📚 Sources:**" | ||
| for i, result in enumerate(search_results[:3]): | ||
| title = result.get("title", "Source") | ||
| url = result.get("url", "#") | ||
| formatted += f"\n{i+1}. [{title}]({url})" | ||
| return formatted | ||
|
|
||
| def _generate_fallback_response(self, message: str) -> str: | ||
| """Return a friendly fallback message when no data is found.""" | ||
| return ( | ||
| f"I’d love to help you learn about {ORG_NAME}, but I couldn’t find current information for your question:\n" | ||
| f"“{message}”\n\n" | ||
| "This might be because:\n" | ||
| "- The information isn’t publicly available yet\n" | ||
| "- The search terms need to be more specific\n" | ||
| "- There might be temporary connectivity issues\n\n" | ||
| f"Try asking a more specific question, or visit our official website and documentation for the most up-to-date info about {ORG_NAME}." | ||
| ) | ||
|
|
||
| async def handle_faq_node(state: AgentState, search_tool: Any, llm: Any) -> dict: | ||
| """ | ||
| Legacy compatibility wrapper for backward support. | ||
| Use FAQTool.get_response() directly in new code. | ||
| """ | ||
| tool = FAQTool(search_tool, llm) | ||
| return await tool.get_response(state) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Significant code duplication with faq.py handler.
The FAQTool class in this file duplicates substantial logic from backend/app/agents/devrel/nodes/handlers/faq.py. Both implement:
_dynamic_faq_processwith the same 5-step pipeline_refine_faq_querywith similar refinement logic_synthesize_faq_responsewith identical synthesis approach_format_faq_responsefor source citations_generate_fallback_responsefor error handling
This violates DRY principles and creates maintenance burden. If business logic changes, both files must be updated.
Consider consolidating into a single implementation. Options:
- Keep FAQTool only: Remove the handler functions from
faq.pyand useFAQTooleverywhere - Keep handler only: Remove
FAQTooland use the handler functions directly - Composition: Have one delegate to the other
Based on the architecture, option 1 seems cleanest since FAQTool is a proper class with encapsulated state.
🧰 Tools
🪛 Ruff (0.13.3)
89-96: Consider moving this statement to an else block
(TRY300)
98-98: Do not catch blind exception: Exception
(BLE001)
99-99: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
121-122: try-except-continue detected, consider logging the exception
(S112)
121-121: Do not catch blind exception: Exception
(BLE001)
146-146: Do not catch blind exception: Exception
(BLE001)
147-147: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
154-154: Comment contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF003)
162-162: Do not catch blind exception: Exception
(BLE001)
163-163: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
224-224: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
224-224: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
227-227: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
| refinement_prompt = load_prompt("refinement_prompt").format( | ||
| org_name=ORG_NAME, message=message |
There was a problem hiding this comment.
🧩 Analysis chain
Verify prompt name mapping between snake_case and UPPER_CASE.
The code calls load_prompt("refinement_prompt") and load_prompt("synthesis_prompt") (line 193), but the constants in faq_prompt.py are named REFINEMENT_PROMPT and SYNTHESIS_PROMPT. After fixing the load_prompt function, ensure the name mapping logic correctly converts between these formats.
Run this script to verify the prompt constants exist:
🏁 Script executed:
#!/bin/bash
# Check if the expected prompt constants exist in faq_prompt.py
rg -n "^(REFINEMENT_PROMPT|SYNTHESIS_PROMPT)\s*=" backend/app/agents/devrel/prompts/faq_prompt.pyLength of output: 51
🏁 Script executed:
#!/bin/bash
# Locate load_prompt definition across the repository
rg -n "def load_prompt" -R .Length of output: 404
🏁 Script executed:
#!/bin/bash
# Search for load_prompt definition in Python files
rg -n "def load_prompt" --type pyLength of output: 81
🏁 Script executed:
#!/bin/bash
# Display implementation of load_prompt for mapping logic inspection
rg -n -C5 "def load_prompt" backend/app/agents/devrel/tools/faq_tool.pyLength of output: 359
🏁 Script executed:
#!/bin/bash
# Dump load_prompt implementation for mapping inspection
sed -n '23,120p' backend/app/agents/devrel/tools/faq_tool.pyLength of output: 3330
Fix fallback in load_prompt to correctly map snake_case to UPPER_CASE constants. The current code only imports REFINEMENT_PROMPT and does getattr(REFINEMENT_PROMPT, name) (which always fails). Instead import the faq_prompt module and use something like getattr(faq_prompt, name.upper()) so that "refinement_prompt"→REFINEMENT_PROMPT and "synthesis_prompt"→SYNTHESIS_PROMPT.
🤖 Prompt for AI Agents
In backend/app/agents/devrel/tools/faq_tool.py around lines 169-170, the call to
load_prompt currently tries to retrieve attributes from an imported
REFINEMENT_PROMPT object which always fails; change load_prompt to import the
faq_prompt module and lookup the constant by converting the snake_case name to
UPPER_CASE (e.g., use getattr(faq_prompt, name.upper())) so "refinement_prompt"
-> REFINEMENT_PROMPT and "synthesis_prompt" -> SYNTHESIS_PROMPT, and ensure the
module attribute is returned rather than attempting getattr on a single prompt
constant.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/app/agents/devrel/tools/faq_tool.py (1)
22-42: Critical prompt loading issues remain unresolved.The past review comments extensively documented two blocking defects that prevent the dynamic FAQ pipeline from functioning:
Line 22:
FAQ_PROMPTS_PATHpoints to the Python filefaq_prompt.pyrather than the prompts directory, so line 28'sprompt_path = FAQ_PROMPTS_PATH / f"{name}.txt"constructs invalid paths like.../faq_prompt.py/refinement_prompt.txt.Lines 37-38: The fallback imports
REFINEMENT_PROMPT(a string constant) then callsgetattr(REFINEMENT_PROMPT, name), which always raisesAttributeErrorbecause you cannot callgetattron a string.Since
load_promptcannot return a valid prompt string, every call to_refine_faq_query(line 219) and_synthesize_faq_response(line 243) will throwFileNotFoundError, short-circuiting the dynamic FAQ flow into the fallback response for every request.Apply this diff to fix both issues:
-FAQ_PROMPTS_PATH = Path(__file__).parent / "prompt" / "faq_prompt.py" +FAQ_PROMPTS_DIR = Path(__file__).parent.parent / "prompts" def load_prompt(name: str) -> str: """ Load a prompt from a .txt file or fallback to faq_prompt.py. """ - prompt_path = FAQ_PROMPTS_PATH / f"{name}.txt" + prompt_path = FAQ_PROMPTS_DIR / f"{name}.txt" # 1. Try to load from text file if prompt_path.exists(): with open(prompt_path, "r", encoding="utf-8") as f: return f.read() # 2. Fallback: Load from Python module try: - from app.agents.devrel.prompts.faq_prompt import REFINEMENT_PROMPT - return getattr(REFINEMENT_PROMPT, name) - except (ImportError, AttributeError): + from app.agents.devrel.prompts import faq_prompt as prompt_module + # Convert snake_case to UPPER_CASE constant name + constant_name = name.upper() + return getattr(prompt_module, constant_name) + except (ImportError, AttributeError) as err: raise FileNotFoundError( f"Prompt '{name}' not found in {prompt_path} or faq_prompt.py" - ) + ) from err
🧹 Nitpick comments (5)
backend/app/agents/devrel/tools/faq_tool.py (5)
106-115: Improve exception handling and error exposure.The broad
Exceptioncatch exposes the raw error message to the user (line 111), which may leak implementation details or sensitive information. Additionally, usinglogging.exceptioninstead oflogging.errorwould automatically include the stack trace.Apply this diff to improve the exception handling:
except Exception as e: - logger.error(f"[FAQ_TOOL] Failed to handle FAQ: {e}") + logger.exception("[FAQ_TOOL] Failed to handle FAQ") return { "task_result": { "type": "faq", - "response": f"Sorry, something went wrong while handling your FAQ request: {e}", + "response": "I apologize, but I encountered an error while processing your FAQ request. Please try again or contact support if the issue persists.", "source": "error", }, "current_task": "faq_failed", }
118-132: Consider logging malformed official handles.The silent
try-except-continue(lines 124-130) makes it difficult to detect configuration issues ifOFFICIAL_HANDLEScontains malformed URLs. Adding debug-level logging would help administrators identify and fix invalid configurations.Apply this diff to add debug logging:
for u in OFFICIAL_HANDLES: try: parsed = urlparse(u) domain = parsed.netloc or parsed.path if domain: domains.append(domain) - except Exception: - continue + except Exception as e: + logger.debug(f"[FAQ_TOOL] Skipping malformed handle '{u}': {e}") + continue
159-161: Uselogging.exceptionfor automatic stack trace inclusion.Replace
logging.errorwithlogging.exceptionto automatically capture the full exception context, which aids in debugging search failures.Based on static analysis hints.
Apply this diff:
except Exception as err: - logger.error(f"[FAQ_TOOL] Search failed: {err}") + logger.exception("[FAQ_TOOL] Search failed") return self._generate_fallback_response(message)
175-177: Uselogging.exceptionfor automatic stack trace inclusion.Replace
logging.errorwithlogging.exceptionto automatically capture the full exception context, which aids in debugging pipeline failures.Based on static analysis hints.
Apply this diff:
except Exception as e: - logger.error(f"[FAQ_TOOL] Error in dynamic FAQ process: {e}") + logger.exception("[FAQ_TOOL] Error in dynamic FAQ process") return self._generate_fallback_response(message)
188-212: Consider logging malformed URLs during filtering.The silent
try-except-continueblocks (lines 194-195, 211-212) make it difficult to detect when search results contain malformed URLs. Adding debug-level logging would help identify data quality issues.Apply this diff to add debug logging:
for handle in OFFICIAL_HANDLES: try: parsed = urlparse(handle) domain = parsed.netloc or parsed.path if domain: allowed_domains.add(domain.lower()) - except Exception: - continue + except Exception as e: + logger.debug(f"[FAQ_TOOL] Skipping malformed handle in filtering '{handle}': {e}") + continue if not allowed_domains: return results # Filter results filtered = [] for result in results: url = result.get("url", "") try: parsed = urlparse(url) result_domain = parsed.netloc.lower() # Check if domain matches any allowed domain if any( result_domain == allowed or result_domain.endswith(f".{allowed}") for allowed in allowed_domains ): filtered.append(result) - except Exception: - continue + except Exception as e: + logger.debug(f"[FAQ_TOOL] Skipping result with malformed URL '{url}': {e}") + continue
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/app/agents/devrel/agent.py(2 hunks)backend/app/agents/devrel/tools/faq_tool.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/agents/devrel/agent.py
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/agents/devrel/tools/faq_tool.py (3)
backend/app/agents/state.py (1)
AgentState(18-73)backend/app/agents/devrel/tools/search_tool/tavilly.py (1)
TavilySearchTool(10-52)backend/app/agents/devrel/nodes/handlers/faq.py (6)
_generate_fallback_response(216-228)_dynamic_faq_process(77-126)_refine_faq_query(129-154)_synthesize_faq_response(157-197)_format_faq_response(200-213)handle_faq_node(17-74)
🪛 Ruff (0.14.0)
backend/app/agents/devrel/tools/faq_tool.py
40-42: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
40-42: Avoid specifying long messages outside the exception class
(TRY003)
97-104: Consider moving this statement to an else block
(TRY300)
106-106: Do not catch blind exception: Exception
(BLE001)
107-107: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
129-130: try-except-continue detected, consider logging the exception
(S112)
129-129: Do not catch blind exception: Exception
(BLE001)
159-159: Do not catch blind exception: Exception
(BLE001)
160-160: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
175-175: Do not catch blind exception: Exception
(BLE001)
176-176: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
194-195: try-except-continue detected, consider logging the exception
(S112)
194-194: Do not catch blind exception: Exception
(BLE001)
211-212: try-except-continue detected, consider logging the exception
(S112)
211-211: Do not catch blind exception: Exception
(BLE001)
| if any(allowed in result_domain or result_domain in allowed for allowed in allowed_domains): | ||
| filtered.append(result) |
There was a problem hiding this comment.
Domain matching logic may allow unintended subdomain matches.
The bidirectional substring check allowed in result_domain or result_domain in allowed could match unintended domains. For example, if allowed is "example.com", then "example.com.attacker.com" would match because "example.com" in "example.com.attacker.com" is true.
Apply this diff to use exact domain or proper subdomain matching:
# Check if domain matches any allowed domain
- if any(allowed in result_domain or result_domain in allowed for allowed in allowed_domains):
+ # Match exact domain or proper subdomain (e.g., "blog.example.com" matches "example.com")
+ if any(
+ result_domain == allowed or result_domain.endswith(f".{allowed}")
+ for allowed in allowed_domains
+ ):
filtered.append(result)📝 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.
| if any(allowed in result_domain or result_domain in allowed for allowed in allowed_domains): | |
| filtered.append(result) | |
| # Check if domain matches any allowed domain | |
| # Match exact domain or proper subdomain (e.g., "blog.example.com" matches "example.com") | |
| if any( | |
| result_domain == allowed or result_domain.endswith(f".{allowed}") | |
| for allowed in allowed_domains | |
| ): | |
| filtered.append(result) |
🤖 Prompt for AI Agents
In backend/app/agents/devrel/tools/faq_tool.py around lines 209-210, the current
bidirectional substring check can produce false positives (e.g., "example.com"
matching "example.com.attacker.com"); replace it with a proper domain check:
normalize both domains to lowercase and strip any trailing dots, then accept
when result_domain == allowed or result_domain.endswith("." + allowed) to allow
only exact matches or legitimate subdomains; update the condition accordingly
and ensure allowed_domains is iterated as before.
| async def handle_faq_node(state: AgentState, llm: Any) -> dict: | ||
| """ | ||
| Legacy compatibility wrapper for backward support. | ||
| Use FAQTool.get_response() directly in new code. | ||
| """ | ||
| tool = FAQTool(llm) | ||
| return await tool.get_response(state) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify signature compatibility with handler in faq.py.
The wrapper signature handle_faq_node(state: AgentState, llm: Any) differs from the handler in backend/app/agents/devrel/nodes/handlers/faq.py which expects handle_faq_node(state: AgentState, search_tool: Any, llm: Any). While the docstring indicates this is for "backward support," the signature mismatch could cause TypeError if callers pass search_tool.
Run this script to check all call sites:
🏁 Script executed:
#!/bin/bash
# Find all calls to handle_faq_node to verify parameter usage
rg -n -B2 -A2 'handle_faq_node\s*\(' --type=pyLength of output: 1813
Align handle_faq_node signature with callers
Update the wrapper in backend/app/agents/devrel/tools/faq_tool.py to:
async def handle_faq_node(state: AgentState, search_tool: Any, llm: Any) -> dict:
"""
Legacy compatibility wrapper for backward support.
Use FAQTool.get_response() directly in new code.
"""
tool = FAQTool(llm)
return await tool.get_response(state)This matches calls in tool_wrappers.py and nodes/handlers/faq.py.
🤖 Prompt for AI Agents
In backend/app/agents/devrel/tools/faq_tool.py around lines 283 to 289, the
async wrapper handle_faq_node has the wrong signature (missing search_tool)
compared to its callers; update the function signature to accept (state:
AgentState, search_tool: Any, llm: Any) and leave the body unchanged so it
constructs FAQTool with llm and calls get_response(state); ensure imports/types
still satisfy the new parameter and run tests to confirm no other call sites
need changes.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
backend/app/agents/devrel/nodes/react_supervisor.py (2)
35-37: Single source of truth for iteration cap; drop router check.Stop condition is enforced in react_supervisor_node (>=). Remove duplicate router check to avoid drift and off‑by‑one confusion.
@@ - # Safety check for max iterations - if iteration_count >= MAX_ITERATIONS: + # Safety check for max iterations + if iteration_count >= MAX_ITERATIONS: logger.warning(f"Max iterations ({MAX_ITERATIONS}) reached for session {state.session_id}") return _create_completion_response(state, "Maximum iterations reached") @@ - iteration_count = state.context.get("iteration_count", 0) - if iteration_count > MAX_ITERATIONS: - logger.warning(f"Max iterations reached for session {state.session_id}") - return "complete" + iteration_count = state.context.get("iteration_count", 0) # Node enforces capAlso applies to: 183-187
251-271: History cap: confirm requirement vs summarization.You cap to MAX_CONVERSATION_HISTORY=5. Product feedback suggested reverting reliance on caps due to working summarization and large context windows. Confirm desired behavior before merging.
Would you like to remove/relax the cap, or make it configurable via settings with a sensible default?
🧹 Nitpick comments (8)
backend/app/agents/devrel/tool_wrappers.py (2)
18-24: Type hints for clarity on tool and LLM dependencies.Annotate parameters to document the contract and improve IDE help.
-async def faq_handler_tool_node(state: AgentState, search_tool, llm) -> Dict[str, Any]: +async def faq_handler_tool_node(state: AgentState, search_tool: Any, llm: Any) -> Dict[str, Any]:
50-54: Reuse shared latest-message helper for consistency.Use the existing util to avoid drift with other nodes.
+from .nodes.generate_response import _get_latest_message as get_latest_message @@ - latest_message = ( - state.messages[-1].get("content", "") - if state.messages else - state.context.get("original_message", "") - ) + latest_message = get_latest_message(state)backend/app/agents/devrel/nodes/react_supervisor.py (6)
39-48: Bound tool_results included in the prompt to prevent prompt bloat.Cap entries before serialization; keep most recent N.
- try: + try: + # Limit to last 5 tool results to keep prompt small + if tool_results and isinstance(tool_results, list): + tool_results = tool_results[-5:] if tool_results: tool_results_str = json.dumps(tool_results, indent=2, default=str) else: tool_results_str = "No previous tool results"
115-117: Prefer logger.exception (and avoid f-strings) for tracebacks.Cleaner and satisfies RUF010; includes stacktrace automatically.
- except Exception as e: - logger.error(f"Error in react_supervisor_node: {e}", exc_info=True) - return _create_error_response(state, f"Supervisor error: {str(e)}") + except Exception: + logger.exception("Error in react_supervisor_node") + return _create_error_response(state, "Supervisor error")
155-160: Parser: accept REASONING: alias.LLMs often emit REASONING:. Handle both.
- elif line.startswith("REASON:"): + elif line.startswith(("REASON:", "REASONING:")): if current_section and content_buffer: decision[current_section] = " ".join(content_buffer) - current_section = "reasoning" - content_buffer = [line.replace("REASON:", "").strip()] + current_section = "reasoning" + content_buffer = [line.split(":", 1)[1].strip()]
226-235: Minor: UTC timestamps and list concat nit in add_tool_result.Use timezone-aware ISO timestamps and avoid + concat (RUF005).
-from datetime import datetime +from datetime import datetime, timezone @@ - tool_entry = { + tool_entry = { "tool": tool_name, "result": result, "iteration": state.context.get("iteration_count", 0), - "timestamp": datetime.now().isoformat() + "timestamp": datetime.now(timezone.utc).isoformat() } @@ - tools_used = getattr(state, 'tools_used', []) + [tool_name] + tools_used = [*getattr(state, 'tools_used', []), tool_name]
45-45: Broadexcept Exceptionused in many places.Narrow where feasible or document rationale. For irreducible cases, prefer logger.exception for automatic tracebacks.
Also applies to: 116-116, 171-172, 195-197, 237-239, 247-249, 269-271, 287-289
98-103: Minor logging style.Use parameterized logging to avoid eager f-string formatting.
-logger.debug(f"Current iteration: {iteration_count}") -logger.debug(f"Latest message length: {len(latest_message or '')}") -logger.info(f"ReAct Supervisor decision: {decision['action']}") -logger.debug(f"Supervisor thinking: {decision.get('thinking', '')[:100]}...") -logger.debug(f"Supervisor reasoning: {decision.get('reasoning', '')[:100]}...") +logger.debug("Current iteration: %d", iteration_count) +logger.debug("Latest message length: %d", len(latest_message or "")) +logger.info("ReAct Supervisor decision: %s", decision["action"]) +logger.debug("Supervisor thinking: %s", decision.get("thinking", "")[:100] + "...") +logger.debug("Supervisor reasoning: %s", decision.get("reasoning", "")[:100] + "...")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/app/agents/devrel/nodes/react_supervisor.py(2 hunks)backend/app/agents/devrel/tool_wrappers.py(3 hunks)backend/app/core/config/settings.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/core/config/settings.py
🧰 Additional context used
🧬 Code graph analysis (2)
backend/app/agents/devrel/tool_wrappers.py (2)
backend/app/agents/devrel/tools/faq_tool.py (1)
handle_faq_node(283-289)backend/app/agents/devrel/nodes/handlers/faq.py (1)
handle_faq_node(17-74)
backend/app/agents/devrel/nodes/react_supervisor.py (2)
backend/app/agents/state.py (1)
AgentState(18-77)backend/app/agents/devrel/nodes/generate_response.py (1)
_get_latest_message(102-106)
🪛 Ruff (0.14.1)
backend/app/agents/devrel/nodes/react_supervisor.py
45-45: Do not catch blind exception: Exception
(BLE001)
117-117: Use explicit conversion flag
Replace with conversion flag
(RUF010)
193-193: Consider moving this statement to an else block
(TRY300)
226-226: Consider [*getattr(state, 'tools_used', []), tool_name] instead of concatenation
Replace with [*getattr(state, 'tools_used', []), tool_name]
(RUF005)
228-235: Consider moving this statement to an else block
(TRY300)
285-285: Consider moving this statement to an else block
(TRY300)
| from app.agents.state import AgentState | ||
| from .nodes.react_supervisor import add_tool_result | ||
| from .nodes.handlers.faq import handle_faq_node | ||
| from app.agents.devrel.tools.faq_tool import handle_faq_node |
There was a problem hiding this comment.
🧩 Analysis chain
Fix import/signature mismatch for FAQ handler (runtime TypeError).
You import handle_faq_node from tools.faq_tool (signature: (state, llm)) but call it with (state, search_tool, llm). Import the handler that accepts search_tool, or adapt the call.
Apply one of the following:
Option A — import the dynamic handler:
-from app.agents.devrel.tools.faq_tool import handle_faq_node
+from .nodes.handlers.faq import handle_faq_nodeOption B — keep tools.faq_tool and adapt call:
- handler_result = await handle_faq_node(state, search_tool, llm)
+ handler_result = await handle_faq_node(state, llm) # search_tool unused by tools.faq_tool wrapperPrefer A to actually pass search_tool to the dynamic flow. Based on learnings
Run to confirm definitions/callers:
Also applies to: 22-23
🏁 Script executed:
#!/bin/bash
rg -nP "def\s+handle_faq_node\s*\(" -C2 backend/app/agents/devrel
rg -nP "\bhandle_faq_node\s*\(" -C2 backend/app/agents/devrelLength of output: 2604
Fix import/signature mismatch for FAQ handler (runtime TypeError).
You import handle_faq_node from tools.faq_tool (signature: (state, llm)) but call it with (state, search_tool, llm). Import the dynamic handler instead:
-from app.agents.devrel.tools.faq_tool import handle_faq_node
+from app.agents.devrel.nodes.handlers.faq import handle_faq_nodeThis passes search_tool to the actual implementation that requires it.
📝 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.
| from app.agents.devrel.tools.faq_tool import handle_faq_node | |
| from app.agents.devrel.nodes.handlers.faq import handle_faq_node |
🤖 Prompt for AI Agents
In backend/app/agents/devrel/tool_wrappers.py around line 5, the file imports
handle_faq_node (signature (state, llm)) but the code calls it with (state,
search_tool, llm), causing a TypeError; replace the current import with the
dynamic FAQ handler/factory exported by app.agents.devrel.tools.faq_tool that
accepts the search_tool parameter (or returns a handler that closes over
search_tool), and update the import statement accordingly so the invoked
function receives (state, search_tool, llm).
|
Could you please look up and try the codebase once? I'm facing an error. PS: please try restructuring a few of your commits, a lot of them are overlapping |
| return "" | ||
|
|
||
|
|
||
| def _get_conversation_history(state: AgentState, max_messages: int = MAX_CONVERSATION_HISTORY) -> str: |
There was a problem hiding this comment.
This function was already defined in the codebase.
| return {"context": state.context if hasattr(state, 'context') else {}} | ||
|
|
||
|
|
||
| def _get_latest_message(state: AgentState) -> str: |
There was a problem hiding this comment.
This function was already defined in the codebase.
| if not latest_message: | ||
| logger.warning("[FAQ_HANDLER] Empty latest user message; returning fallback") | ||
| return { | ||
| "task_result": { | ||
| "type": "faq", | ||
| "response": _generate_fallback_response(latest_message, ORG_NAME), | ||
| "source": "dynamic_web_search" | ||
| }, | ||
| "current_task": "faq_handled" | ||
| } |
There was a problem hiding this comment.
not sure if this section is really required...
There was a problem hiding this comment.
faq.py is not required now..i have merged its functions with faq_tool.py..Will remove the commit
| return refined_query | ||
|
|
||
|
|
||
| async def _synthesize_faq_response( |
There was a problem hiding this comment.
Is this really required? We already have a response synthesizer prompt
| return formatted_response | ||
|
|
||
|
|
||
| def _generate_fallback_response(message: str, org_name: str) -> str: |
There was a problem hiding this comment.
Not required. In case of anomaly where there won't be any results the bot itself manages that scenario and responds well.
| ### Your Response: | ||
| """ | ||
| SYNTHESIS_PROMPT = """ |
There was a problem hiding this comment.
This is duplicated in your PR.
2 participants
🌟 Feature Implementation: Dynamic FAQ Handler
Problem Solved
Key Features Implemented
✅ Smart Query Routing: ReAct Supervisor intelligently identifies organizational queries
✅ Dynamic Web Search: Real-time information retrieval from organization's public presence
✅ Automated Synthesis: Converts search results into comprehensive, user-friendly responses
✅ Source Attribution: Provides links for users to explore topics further
Testing Results
Successfully tested with query: "How can i contribute to AOSSIE Projects?"
Technical Implementation
Ready for review and production deployment.
Closes #97
bandicam.2025-08-10.19-47-44-389.mp4
Summary by CodeRabbit
New Features
Enhancements
Configuration
Chores