src/backend/chat/tools/document_search_rag.py (1)

27-31: Move the guard before instantiating the document store.

The collection_id check on line 29 happens after the document_store is already instantiated on line 27. This is inefficient since the backend is created unnecessarily when there's no collection to search.

Apply this diff to optimize the control flow:

- document_store_backend = import_string(settings.RAG_DOCUMENT_SEARCH_BACKEND)
-
- document_store = document_store_backend(ctx.deps.conversation.collection_id)
-
if not ctx.deps.conversation.collection_id:
return ToolReturn(return_value=[], content="No documents to search in yet.")

+ document_store_backend = import_string(settings.RAG_DOCUMENT_SEARCH_BACKEND)
+ document_store = document_store_backend(ctx.deps.conversation.collection_id)
+
rag_results = document_store.search(query)

src/backend/chat/clients/pydantic_ai.py (1)

406-406: Accessing protected member _function_toolset is fragile.

This relies on an internal implementation detail of pydantic_ai.Agent. If the library updates its internals, this check will break silently or raise an AttributeError.

Consider wrapping this in a try-except or using a more robust approach:

- fetch_url_exists = "fetch_url" in self.conversation_agent._function_toolset.tools # pylint: disable=protected-access
+ # Check if fetch_url tool already exists
+ try:
+ fetch_url_exists = "fetch_url" in self.conversation_agent._function_toolset.tools # pylint: disable=protected-access
+ except AttributeError:
+ fetch_url_exists = False

Alternatively, consider maintaining a local set of dynamically added tool names to avoid relying on internals.

src/backend/chat/tools/__init__.py (1)

21-27: Clarify the purpose of this registry entry.

The comment states fetch_url is added dynamically in pydantic_ai.py and "won't be used via prepare". If so, this entry may be redundant or could cause confusion.

Consider either:

1. Removing this entry if it's truly unused (dynamic registration handles everything)
2. Using a prepare function to conditionally enable it (similar to web search tools)
3. Updating the comment to explain why it's kept here (e.g., for discoverability, testing, or future configuration-based enablement)

src/backend/chat/tools/fetch_url.py (2)

26-28: Simplify and fix the URL regex pattern.

Static analysis flagged several issues with this regex:

• [$-_@.&+] is an overly permissive range matching ASCII 36-95 (includes %&'()*+,-./:;<=>?@ etc.)
• Duplicate characters and unnecessary grouping
• [0-9] should use \d

Consider using a simpler, more robust URL pattern:

"\'\])]+' +)">-URL_PATTERN = re.compile(
- r'http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\\(\\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+'
-)
+URL_PATTERN = re.compile(
+ r'https?://[^\s<>"\'\])]+'
+)

Or for stricter matching:

URL_PATTERN = re.compile(
r'https?://(?:[\w\-._~:/?#\[\]@!$&\'()*+,;=%]|%[0-9A-Fa-f]{2})+'
)

---

62-96: Consider simplifying _extract_text_from_message to reduce cognitive complexity.

Static analysis flags cognitive complexity of 24 (limit: 15). While the logic handles multiple message formats correctly, extracting helper functions could improve readability.

str: - ... + text_parts = [] + if hasattr(message, 'parts'): + text_parts.extend(_extract_text_from_parts(message.parts, is_dict=False)) + if hasattr(message, 'content') and message.content: + text_parts.append(message.content) + elif isinstance(message, dict): + text_parts.extend(_extract_text_from_parts(message.get('part s', []), is_dict=True)) + if content := message.get('content', ''): + text_parts.append(content) + return ' '.join(text_parts)">+def _extract_text_from_parts(parts, is_dict: bool) -> list[str]:
+ """Extract text from message parts."""
+ texts = []
+ for part in parts or []:
+ if is_dict:
+ if isinstance(part, dict) and part.get('type') == 'text' and part.get('text'):
+ texts.append(part['text'])
+ elif isinstance(part, TextUIPart) and part.text:
+ texts.append(part.text)
+ return texts
+
+
def _extract_text_from_message(message) -> str:
- ...
+ text_parts = []
+ if hasattr(message, 'parts'):
+ text_parts.extend(_extract_text_from_parts(message.parts, is_dict=False))
+ if hasattr(message, 'content') and message.content:
+ text_parts.append(message.content)
+ elif isinstance(message, dict):
+ text_parts.extend(_extract_text_from_parts(message.get('part s', []), is_dict=True))
+ if content := message.get('content', ''):
+ text_parts.append(content)
+ return ' '.join(text_parts)

src/backend/chat/tests/tools/test_fetch_url.py (1)

169-169: Remove unused mock_storage variable.

Static analysis correctly flags that mock_storage is captured but never used in assertions. If storage behavior doesn't need verification, use _ to indicate intentional ignorance.

- with patch("chat.tools.fetch_url.import_string") as mock_import, patch(
- "chat.tools.fetch_url.models.ChatConversationAttachment.obje cts.acreate", new_callable=AsyncMock
- ) as mock_attachment_create, patch("chat.tools.fetch_url.default_storage.save") as mock_storage:
+ with patch("chat.tools.fetch_url.import_string") as mock_import, patch(
+ "chat.tools.fetch_url.models.ChatConversationAttachment.obje cts.acreate", new_callable=AsyncMock
+ ) as mock_attachment_create, patch("chat.tools.fetch_url.default_storage.save"):

Or add assertions on storage if that behavior should be verified.

Also applies to: 273-273

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/backend/chat/tools/fetch_url.py (1)

28-31: Regex character range is overly permissive.

The character class [$-_@.&+] creates a range from $ (ASCII 36) to _ (ASCII 95), which unintentionally includes digits, uppercase letters, and various punctuation. Consider using an explicit character class or a well-tested URL regex pattern.

"\')\]}>]+' +)">-URL_PATTERN = re.compile(
- r'http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\\(\\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+'
-)
+URL_PATTERN = re.compile(
+ r'https?://[^\s<>"\')\]}>]+'
+)

Alternatively, use a more robust approach with urllib.parse for validation after extraction.

src/backend/chat/tools/fetch_url.py (4)

65-99: Consider reducing cognitive complexity.

Static analysis flags this function for high cognitive complexity (24 vs 15 allowed). The nested conditionals for different message types can be simplified.

list[str]: + """Extract text parts from a dict-based message.""" + parts = [] + for part in message.get('parts', []): + if isinstance(part, dict) and part.get('type') == 'text' and part.get('text'): + parts.append(part['text']) + if content := message.get('content'): + parts.append(content) + return parts + + def _extract_text_from_message(message) -> str: - ... + if hasattr(message, 'parts'): + text_parts = _extract_text_from_ui_message(message) + elif isinstance(message, dict): + text_parts = _extract_text_from_dict(message) + else: + text_parts = [] + return ' '.join(text_parts)">+def _extract_text_from_ui_message(message) -> list[str]:
+ """Extract text parts from a UIMessage object."""
+ parts = []
+ for part in getattr(message, 'parts', None) or []:
+ if isinstance(part, TextUIPart) and part.text:
+ parts.append(part.text)
+ if content := getattr(message, 'content', None):
+ parts.append(content)
+ return parts
+
+
+def _extract_text_from_dict(message: dict) -> list[str]:
+ """Extract text parts from a dict-based message."""
+ parts = []
+ for part in message.get('parts', []):
+ if isinstance(part, dict) and part.get('type') == 'text' and part.get('text'):
+ parts.append(part['text'])
+ if content := message.get('content'):
+ parts.append(content)
+ return parts
+
+
def _extract_text_from_message(message) -> str:
- ...
+ if hasattr(message, 'parts'):
+ text_parts = _extract_text_from_ui_message(message)
+ elif isinstance(message, dict):
+ text_parts = _extract_text_from_dict(message)
+ else:
+ text_parts = []
+ return ' '.join(text_parts)

---

124-126: Remove redundant regex search.

URL_PATTERN.search() is called before findall(), but findall() already returns an empty list if there are no matches. This adds unnecessary overhead.

- if text_content and URL_PATTERN.search(text_content):
- matches = URL_PATTERN.findall(text_content)
- found_urls.update(matches)
+ if text_content:
+ found_urls.update(URL_PATTERN.findall(text_content))

---

140-183: Consider adding delay for rate-limit retries.

The retry logic correctly handles 403/429 status codes, but retrying immediately after a 429 (rate limit) may not help. Consider adding a small delay (e.g., asyncio.sleep(1)) before retrying on 429.

---

256-418: Consider extracting helper functions to reduce complexity.

This function has high cognitive complexity (23 vs 15 allowed) due to handling multiple content types and special cases. Consider extracting the docs.numerique.gouv.fr logic and general HTTP fetch logic into separate async helper functions.

Suggested structure:

async def _fetch_docs_numerique(client, url, conversation, user) -> ToolReturn | None:
"""Handle docs.numerique.gouv.fr URLs. Returns None if not applicable."""
...

async def _fetch_general_url(client, url, conversation, user) -> ToolReturn:
"""Handle general URL fetching with trafilatura extraction."""
...

async def fetch_url(ctx: RunContext, url: str) -> ToolReturn:
# Security check
# Try docs.numerique.gouv.fr handler
# Fall back to general handler
...

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/backend/chat/tools/fetch_url.py (2)

187-254: Blocking I/O in async function.

This async function contains multiple synchronous blocking I/O operations that will block the event loop:

1. Lines 226-234: document_store.parse_document() and document_store.store_document() make synchronous HTTP requests to the Albert API using requests.post().
2. Line 250: default_storage.save() performs synchronous file I/O.

Under load, these blocking calls will degrade performance and delay other concurrent requests.

Wrap these blocking calls with sync_to_async:

+from asgiref.sync import sync_to_async
...
- parsed_content = document_store.parse_document(
+ parsed_content = await sync_to_async(document_store.parse_document)(
name=safe_rag_name,
content_type=content_type,
content=BytesIO(content_bytes),
)

- document_store.store_document(
+ await sync_to_async(document_store.store_document)(
name=url,
content=parsed_content
)
...
- default_storage.save(key, ContentFile(parsed_content.encode("utf8")))
+ await sync_to_async(default_storage.save)(key, ContentFile(parsed_content.encode("utf8")))

---

345-347: Blocking HTML extraction in async function.

trafilatura.extract() is a synchronous operation that parses and extracts content from HTML. For large documents, this can block the event loop and degrade performance under concurrent load.

Wrap the extraction call to run off the event loop:

+from asgiref.sync import sync_to_async
...
- extracted = trafilatura.extract(response.text) or response.text
+ extracted = await sync_to_async(trafilatura.extract)(response.text) or response.text

src/backend/chat/tools/fetch_url.py (4)

30-32: Simplify the regex subpattern.

The grouped subpattern (?:%[0-9a-fA-F]{2}) can be simplified to %[0-9a-fA-F]{2} since the group is not captured or referenced.

Apply this diff:

URL_PATTERN = re.compile(
- r'https?://(?:[a-zA-Z0-9\-._~:/?#\[\]@!$&\'()*+,;=%]|(?:%[0-9a-fA-F]{2}))+'
+ r'https?://(?:[a-zA-Z0-9\-._~:/?#\[\]@!$&\'()*+,;=%]|%[0-9a-fA-F]{2})+'
)

---

66-100: Consider refactoring to reduce cognitive complexity.

The function handles multiple message formats with nested conditionals, resulting in high cognitive complexity. Extracting helper functions for UIMessage and dict handling would improve readability.

Example refactor:

str: """Extract text from UIMessage (Pydantic model).""" text_parts = [] for part in message.parts or []: if isinstance(part, TextUIPart) and part.text: text_parts.append(part.text) if hasattr(message, 'content') and message.content: text_parts.append(message.content) return ' '.join(text_parts) def _extract_from_dict_message(message: dict) -> str: """Extract text from dict-based message.""" text_parts = [] for part in message.get('parts', []): if isinstance(part, dict) and part.get('type') == 'text': if text := part.get('text', ''): text_parts.append(text) if content := message.get('content', ''): text_parts.append(content) return ' '.join(text_parts)">def _extract_text_from_message(message) -> str:
"""Extract all text content from a message."""
if hasattr(message, 'parts'):
return _extract_from_ui_message(message)
elif isinstance(message, dict):
return _extract_from_dict_message(message)
return ''

def _extract_from_ui_message(message) -> str:
"""Extract text from UIMessage (Pydantic model)."""
text_parts = []
for part in message.parts or []:
if isinstance(part, TextUIPart) and part.text:
text_parts.append(part.text)
if hasattr(message, 'content') and message.content:
text_parts.append(message.content)
return ' '.join(text_parts)

def _extract_from_dict_message(message: dict) -> str:
"""Extract text from dict-based message."""
text_parts = []
for part in message.get('parts', []):
if isinstance(part, dict) and part.get('type') == 'text':
if text := part.get('text', ''):
text_parts.append(text)
if content := message.get('content', ''):
text_parts.append(content)
return ' '.join(text_parts)

---

103-138: Consider simplifying the URL detection logic.

The nested function and multiple conditional checks contribute to elevated cognitive complexity. Flattening the structure would improve maintainability.

Example refactor:

def detect_url_in_conversation(conversation) -> list[str]:
"""Detect URLs present in the conversation messages."""
if not conversation:
return []

found_urls = set()

# Collect all message sources
message_sources = []
if hasattr(conversation, 'ui_messages') and conversation.ui_messages:
message_sources.append(conversation.ui_messages)
if hasattr(conversation, 'messages') and conversation.messages:
message_sources.append(conversation.messages)

# Extract URLs from all sources
for messages in message_sources:
for message in messages or []:
if message:
text_content = _extract_text_from_message(message)
if text_content:
found_urls.update(URL_PATTERN.findall(text_content))

if found_urls:
logger.info("URL detected in messages: %s", found_urls)

return list(found_urls)

---

257-419: Consider extracting helper functions to reduce complexity.

The fetch_url function handles multiple concerns: URL validation, special-case docs.numerique.gouv.fr handling, general URL fetching, content classification, and RAG storage decisions. Extracting helpers for docs.numerique.gouv.fr handling and content processing would improve maintainability.

Example structure:

async def fetch_url(ctx: RunContext, url: str) -> ToolReturn:
"""Main entry point - validates and delegates."""
deps = getattr(ctx, "deps", None)
conversation = getattr(deps, "conversation", None)
user = getattr(deps, "user", None)

# Validate URL was detected
if url not in detect_url_in_conversation(conversation):
return _url_not_detected_response(url)

try:
# Special case handling
if DOCS_HOST in url and "/docs/" in url:
return await _fetch_docs_numerique(url, conversation, user)

# General URL handling
return await _fetch_general_url(url, conversation, user)

except httpx.HTTPStatusError as e:
return _http_error_response(url, e)
except httpx.TimeoutException as e:
return _timeout_response(url, e)
except Exception as e:
return _generic_error_response(url, e)

async def _fetch_docs_numerique(url: str, conversation, user) -> ToolReturn:
"""Handle docs.numerique.gouv.fr URLs."""
# Extract existing lines 285-343
...

async def _fetch_general_url(url: str, conversation, user) -> ToolReturn:
"""Handle general URLs."""
# Extract existing lines 345-395
...

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/backend/chat/tools/fetch_url.py (1)

330-334: trafilatura.extract() is still synchronous and blocks the event loop.

This call performs CPU-intensive HTML parsing synchronously within an async function. For large documents, this can block the event loop. Wrap with sync_to_async:

+from asgiref.sync import sync_to_async
...
# Avoid trafilatura on PDFs
if is_pdf:
extracted = ""
else:
- extracted = trafilatura.extract(response.text) or response.text
+ extracted = await sync_to_async(trafilatura.extract)(response.text) or response.text

src/backend/chat/tests/tools/test_fetch_url.py (1)

172-172: Remove unused mock_storage variables.

The mock_storage variables in the patch context managers on lines 172 and 278 are never used. Remove them to clean up the code.

Apply this diff:

with patch("chat.tools.fetch_url.import_string") as mock_import, patch(
"chat.tools.fetch_url.models.ChatConversationAttachment.obje cts.acreate", new_callable=AsyncMock
- ) as mock_attachment_create, patch("chat.tools.fetch_url.default_storage.save") as mock_storage:
+ ) as mock_attachment_create, patch("chat.tools.fetch_url.default_storage.save"):

Also applies to: 278-278

src/backend/chat/agent_rag/document_converter/markitdown.py (1)

33-42: Minor cleanup: Remove unnecessary seek attempt.

The code creates a fresh BytesIO from the extracted bytes, so the seek(0) attempt on the original BytesIO is unnecessary and the conditional if hasattr(content, 'seek') adds complexity without benefit.

Simplify:

if isinstance(content, BytesIO):
- # Read the BytesIO content to bytes
content_bytes = content.read()
- # Reset position if possible (though we create a new BytesIO anyway)
- content.seek(0) if hasattr(content, 'seek') else None
else:
- # content is already bytes
content_io = BytesIO(content)

The normalization logic is correct otherwise.

src/backend/chat/document_storage.py (1)

73-77: Remove unnecessary seek attempt.

Similar to the markitdown.py file, the seek(0) attempt is unnecessary since a new BytesIO is always created from the extracted bytes. This also makes the comment on line 77 obsolete.

Apply this diff:

if isinstance(content, BytesIO):
- # Read the BytesIO content to bytes
content_bytes = content.read()
- # Reset position if possible (though we create a new BytesIO anyway)
- content.seek(0) if hasattr(content, 'seek') else None
elif isinstance(content, bytes):
content_bytes = content

src/backend/chat/tools/fetch_url.py (5)

31-33: URL regex pattern has unnecessary grouping and overly permissive character class.

The regex has an unnecessarily grouped subpattern (?:...) around the entire character class, and the character range includes more characters than typical URL patterns need. Per the static analysis hint, consider simplifying:

URL_PATTERN = re.compile(
- r'https?://(?:[a-zA-Z0-9\-._~:/?#\[\]@!$&\'()*+,;=%]|(?:%[0-9a-fA-F]{2}))+'
+ r'https?://[a-zA-Z0-9\-._~:/?#\[\]@!$&\'()*+,;=%]+'
)

The percent-encoding alternation (?:%[0-9a-fA-F]{2}) is redundant since %, hex digits, and the encoded characters are already in the main class.

---

67-101: Consider reducing cognitive complexity by extracting handlers.

Static analysis flags cognitive complexity of 24 (limit 15). Extract separate handlers for each message type:

list[str]: + """Extract text from a dict-based message.""" + text_parts = [] + for part in message.get('parts', []): + if isinstance(part, dict) and part.get('type') == 'text': + if text := part.get('text', ''): + text_parts.append(text) + if content := message.get('content', ''): + text_parts.append(content) + return text_parts + + def _extract_text_from_message(message) -> str: - """ - Extract all text content from a message. - ... - """ - text_parts = [] - - # Handle UIMessage objects (Pydantic models) + """Extract all text content from a message.""" if hasattr(message, 'parts'): - for part in message.parts or []: - if isinstance(part, TextUIPart) and part.text: - text_parts.append(part.text) - # Also check the deprecated content field - if hasattr(message, 'content') and message.content: - text_parts.append(message.content) - # Handle dict-based messages (JSON deserialized) + return ' '.join(_extract_text_from_ui_message(message)) elif isinstance(message, dict): - # Check parts - parts = message.get('parts', []) - for part in parts: - if isinstance(part, dict) and part.get('type') == 'text': - text = part.get('text', '') - if text: - text_parts.append(text) - # Check deprecated content field - content = message.get('content', '') - if content: - text_parts.append(content) - - return ' '.join(text_parts) + return ' '.join(_extract_text_from_dict_message(message)) + return ''">+def _extract_text_from_ui_message(message) -> list[str]:
+ """Extract text from a UIMessage Pydantic model."""
+ text_parts = []
+ for part in message.parts or []:
+ if isinstance(part, TextUIPart) and part.text:
+ text_parts.append(part.text)
+ if hasattr(message, 'content') and message.content:
+ text_parts.append(message.content)
+ return text_parts
+
+
+def _extract_text_from_dict_message(message: dict) -> list[str]:
+ """Extract text from a dict-based message."""
+ text_parts = []
+ for part in message.get('parts', []):
+ if isinstance(part, dict) and part.get('type') == 'text':
+ if text := part.get('text', ''):
+ text_parts.append(text)
+ if content := message.get('content', ''):
+ text_parts.append(content)
+ return text_parts
+
+
def _extract_text_from_message(message) -> str:
- """
- Extract all text content from a message.
- ...
- """
- text_parts = []
-
- # Handle UIMessage objects (Pydantic models)
+ """Extract all text content from a message."""
if hasattr(message, 'parts'):
- for part in message.parts or []:
- if isinstance(part, TextUIPart) and part.text:
- text_parts.append(part.text)
- # Also check the deprecated content field
- if hasattr(message, 'content') and message.content:
- text_parts.append(message.content)
- # Handle dict-based messages (JSON deserialized)
+ return ' '.join(_extract_text_from_ui_message(message))
elif isinstance(message, dict):
- # Check parts
- parts = message.get('parts', [])
- for part in parts:
- if isinstance(part, dict) and part.get('type') == 'text':
- text = part.get('text', '')
- if text:
- text_parts.append(text)
- # Check deprecated content field
- content = message.get('content', '')
- if content:
- text_parts.append(content)
-
- return ' '.join(text_parts)
+ return ' '.join(_extract_text_from_dict_message(message))
+ return ''

---

146-170: Consider adding a delay between retries for 429 responses.

When retrying on HTTP 429 (rate limit), immediately retrying without delay may not help. Consider adding a small backoff:

+import asyncio
...
if (not should_retry) or is_last_attempt:
raise
+
+ # Small delay before retry, especially useful for rate limits
+ await asyncio.sleep(0.5 * (attempt + 1))

This is optional since the User-Agent rotation may be sufficient for 403 cases.

---

298-303: Hardcoded French strings limit internationalization.

Multiple French strings are embedded in ToolReturn values (lines 278, 298-303, 319, 357-362). If these are displayed to users or if multi-language support is needed, consider using Django's translation utilities:

from django.utils.translation import gettext as _

content=_(
"Le contenu de ce document est volumineux et a ete indexe dans "
"la base de documents de la conversation. ..."
)

If French-only is intentional for this deployment, this can be deferred.

Also applies to: 357-362

---

264-320: Extract docs.numerique.gouv.fr handling to a dedicated function.

This 55-line special case significantly contributes to cognitive complexity (flagged at 20 vs 15 allowed). Per previous review feedback, extract to a separate async function:

ToolReturn: ... try: m = re.search(r"https?://(?:www\.)?docs\.numerique\.gouv\.fr/doc s/([^/?#]+)", url) if m: - docs_id = m.group(1) - url_transformed = ... - # ... 50+ lines + return await _fetch_docs_numerique(url, m.group(1), conversation, user)">+async def _fetch_docs_numerique(
+ url: str,
+ docs_id: str,
+ conversation,
+ user,
+) -> ToolReturn:
+ """Fetch content from docs.numerique.gouv.fr API."""
+ url_transformed = f"https://{DOCS_HOST}/api/v1.0/documents/{docs_id}/content/?content_format=markdown"
+
+ async with httpx.AsyncClient(timeout=settings.FETCH_URL_TIMEOUT, follow_redirects=True) as client:
+ response = await _get_with_retry(client, url_transformed)
+ data = response.json()
+ content = data.get('content', '')
+
+ if not content:
+ return ToolReturn(
+ return_value={"url": url, "error": "Content empty or private", ...},
+ )
+ # ... rest of the logic
+
+
async def fetch_url(ctx: RunContext, url: str) -> ToolReturn:
...
try:
m = re.search(r"https?://(?:www\.)?docs\.numerique\.gouv\.fr/doc s/([^/?#]+)", url)
if m:
- docs_id = m.group(1)
- url_transformed = ...
- # ... 50+ lines
+ return await _fetch_docs_numerique(url, m.group(1), conversation, user)

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/backend/chat/tools/fetch_url.py (2)

328-328: Wrap trafilatura.extract() to avoid blocking the event loop.

trafilatura.extract() is synchronous and performs CPU-intensive HTML parsing. For large documents, this blocks the event loop. This was flagged in a previous review but remains unaddressed.

+from asgiref.sync import sync_to_async
...
- extracted = trafilatura.extract(response.text) or response.text
+ extracted = await sync_to_async(trafilatura.extract)(response.text) or response.text

---

258-314: Extract docs.numerique.gouv.fr handling to a dedicated function.

This special-case logic spans 55+ lines and increases cognitive complexity. Based on learnings, this should be extracted to a separate function to improve maintainability and reduce the complexity of fetch_url().

Apply this pattern:

+async def _fetch_docs_numerique(
+ url: str,
+ docs_id: str,
+ conversation,
+ user,
+) -> ToolReturn:
+ """Fetch content from docs.numerique.gouv.fr via their API."""
+ url_transformed = f"https://{DOCS_HOST}/api/v1.0/documents/{docs_id}/content/?content_format=markdown"
+ # ... rest of docs-specific logic
+
async def fetch_url(ctx: RunContext, url: str) -> ToolReturn:
...
try:
- # Special handling for docs.numerique.gouv.fr
- m = re.search(r"https?://(?:www\.)?docs\.numerique\.gouv\.fr/doc s/([^/?#]+)", url)
- if m:
- docs_id = m.group(1)
- url_transformed = f"https://{DOCS_HOST}/api/v1.0/documents/{docs_id}/content/?content_format=markdown"
- ...
+ m = re.search(r"https?://(?:www\.)?docs\.numerique\.gouv\.fr/doc s/([^/?#]+)", url)
+ if m:
+ return await _fetch_docs_numerique(url, m.group(1), conversation, user)

Based on learnings, this refactor addresses the maintainer's feedback.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/backend/chat/tools/fetch_url.py (1)

244-256: Use pydantic_messages for URL detection instead of frontend messages.

Line 248 retrieves deps.messages (Vercel SDK format for frontend), but per past review feedback, proper conversation history is in deps.pydantic_messages (Pydantic AI format). URL detection should use the correct message source to ensure reliable detection across the conversation history.

Additionally, the "content" field in the error return (line 255) was questioned in past reviews as potentially incorrect for the expected return structure.

Based on learnings, as per past review feedback from maintainers.

Apply this diff:

- messages_for_detection = getattr(deps, "messages", None)
+ messages_for_detection = getattr(deps, "pydantic_messages", None)
urls = detect_url_in_conversation(messages_for_detection)
logger.info("URLs authorized (extracted from messages): %s", urls)

# If messages are provided, enforce URL presence; otherwise skip the check.
if messages_for_detection is not None and url not in urls:
return ToolReturn(
- return_value={"url": url, "error": "URL not detected in conversation", "content" : f"URL {url} not detected in conversation"},
+ return_value={"url": url, "error": "URL not detected in conversation"},
)

src/backend/chat/tools/fetch_url.py (2)

32-34: Consider simplifying the regex subpattern.

The regex contains an unnecessarily grouped subpattern (?:%[0-9a-fA-F]{2}) that could be unwrapped to %[0-9a-fA-F]{2} without changing behavior, making it slightly more readable.

Apply this diff:

URL_PATTERN = re.compile(
- r'https?://(?:[a-zA-Z0-9\-._~:/?#\[\]@!$&\'()*+,;=%]|(?:%[0-9a-fA-F]{2}))+'
+ r'https?://(?:[a-zA-Z0-9\-._~:/?#\[\]@!$&\'()*+,;=%]|%[0-9a-fA-F]{2})+'
)

---

231-397: Consider reducing cognitive complexity.

Static analysis flags this function's cognitive complexity at 20 (threshold 15). While the logic is correct and handles multiple content types and error cases appropriately, consider extracting helper functions for:

• Docs.numerique.gouv.fr special handling (lines 259-315)
• General URL content processing (lines 317-373)

This would improve maintainability and testability.

This is optional given the "Chill" review mode, but would make the code easier to maintain long-term.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro