Feat/Discord Connector #125
Conversation
|
@MuhamadAjiW is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis change introduces full support for a Discord connector across the backend and frontend. It adds Discord as a connector type, implements Discord bot integration for message retrieval and indexing, updates database enums and migrations, extends validation and indexing logic, and provides UI components for adding, editing, and displaying Discord connectors. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebUI
participant BackendAPI
participant DiscordConnector
participant DiscordAPI
participant DB
User->>WebUI: Add Discord Connector (enter bot token)
WebUI->>BackendAPI: POST /connectors (type=DISCORD_CONNECTOR, token)
BackendAPI->>DB: Save connector config
User->>WebUI: Trigger Indexing
WebUI->>BackendAPI: POST /connectors/{id}/index
BackendAPI->>DiscordConnector: Start bot with token
DiscordConnector->>DiscordAPI: Fetch guilds, channels, messages
DiscordAPI-->>DiscordConnector: Return data
DiscordConnector->>BackendAPI: Return messages
BackendAPI->>DB: Store indexed messages as documents
BackendAPI->>WebUI: Indexing complete
User->>WebUI: Query Discord messages
WebUI->>BackendAPI: Search request (type=DISCORD_CONNECTOR)
BackendAPI->>DB: Retrieve Discord documents
BackendAPI->>WebUI: Return search results
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
1 similar comment
|
|
![recurseml[bot]](https://avatars.githubusercontent.com/in/1054195?s=60&v=4){kind=small}
| op.execute(f"CREATE TYPE {CONNECTOR_ENUM} AS ENUM({old_connector_values_sql})") | ||
|
|
||
| # 3. Update the connector table: | ||
| op.execute( |
There was a problem hiding this comment.
Missing DELETE operation for search_source_connectors table in downgrade(). While the code deletes DISCORD_CONNECTOR documents, it doesn't delete the corresponding connector entries. This asymmetry could lead to orphaned data since the SearchSourceConnector table has a unique constraint on connector_type. When downgrading and recreating the enum without DISCORD_CONNECTOR, any existing DISCORD_CONNECTOR entries would violate the type constraint.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| f"ALTER COLUMN {connector_column_name} " | ||
| f"TYPE {CONNECTOR_ENUM} " | ||
| f"USING {connector_column_name}::text::{CONNECTOR_ENUM}" | ||
| ) |
There was a problem hiding this comment.
Unsafe type casting in the ALTER TABLE statement for search_source_connectors. The code attempts to cast connector_type to the new enum type without first verifying that all existing values can be safely cast. If any DISCORD_CONNECTOR entries exist, the cast will fail since the new enum type doesn't include this value. A DELETE operation should be performed before the cast.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
|
😱 Found 2 issues. Time to roll up your sleeves! 😱 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (1)
surfsense_web/app/dashboard/[search_space_id]/connectors/add/discord-connector/page.tsx (1)
1032-1032:⚠️ Potential issueFix Discord channel URL construction.
The URL construction for Discord channels is incorrect. The format
https://discord.com/channels/@me/{channel_id}is for DM channels. For server channels, it should include the guild ID.- url = f"https://discord.com/channels/@me/{channel_id}" + # Note: This requires guild_id from metadata to construct proper URL + guild_id = metadata.get('guild_id', '') + if guild_id and channel_id: + url = f"https://discord.com/channels/{guild_id}/{channel_id}" + elif channel_id: + # Fallback for DM channels + url = f"https://discord.com/channels/@me/{channel_id}"
🧹 Nitpick comments (21)
surfsense_web/components/editConnector/types.ts (1)
34-34: Consider adding Discord bot token format validation for enhanced security.While the current implementation follows the existing pattern, Discord bot tokens have a specific format that could be validated to prevent configuration errors and enhance security.
- DISCORD_BOT_TOKEN: z.string().optional(), + DISCORD_BOT_TOKEN: z.string() + .optional() + .refine(token => !token || token.length >= 59, { + message: "Discord bot token seems too short" + }),Discord bot tokens are typically 59+ characters long and follow a specific format. This validation would help catch configuration errors early.
surfsense_web/app/dashboard/[search_space_id]/connectors/[connector_id]/edit/page.tsx (1)
195-204: Solid implementation following established patterns.The Discord connector edit form correctly follows the same pattern as other connector types and integrates well with the existing form system. The field name matches the backend configuration and the password input type is appropriate for token security.
Consider making the placeholder text more descriptive to match the pattern used by other connectors:
- placeholder="Bot token..." + placeholder="Begins with MTE..."This would provide users with format guidance similar to how Slack shows "Begins with xoxb-..." and Notion shows "Begins with secret_...".
surfsense_backend/app/schemas/search_source_connector.py (3)
84-84: Fix trailing whitespace.Remove the trailing whitespace on this line.
- +🧰 Tools
🪛 Pylint (3.3.7)
[convention] 84-84: Trailing whitespace
(C0303)
94-94: Fix trailing whitespace.Remove the trailing whitespace on this line.
- +🧰 Tools
🪛 Pylint (3.3.7)
[convention] 94-94: Trailing whitespace
(C0303)
99-99: Consider breaking long line for better readability.The line exceeds 100 characters. Consider breaking it for better readability.
- raise ValueError(f"For DISCORD_CONNECTOR connector type, config must only contain these keys: {allowed_keys}") + raise ValueError( + f"For DISCORD_CONNECTOR connector type, config must only contain these keys: {allowed_keys}" + )🧰 Tools
🪛 Pylint (3.3.7)
[convention] 99-99: Line too long (126/100)
(C0301)
surfsense_backend/app/agents/researcher/nodes.py (4)
403-403: Fix trailing whitespace.Remove the trailing whitespace on this line.
- +🧰 Tools
🪛 Pylint (3.3.7)
[convention] 403-403: Trailing whitespace
(C0303)
418-419: Consider line length and protected member access.Two issues to address:
- Line 418 exceeds 100 characters
- Line 419 accesses a protected member
_format_annotationsThe protected member access appears to be intentional and consistent with the pattern used throughout this file for all connectors.
For the line length, consider breaking it:
- streaming_service.only_update_terminal(f"🗨️ Found {len(discord_chunks)} Discord messages related to your query") + streaming_service.only_update_terminal( + f"🗨️ Found {len(discord_chunks)} Discord messages related to your query" + )🧰 Tools
🪛 Pylint (3.3.7)
[convention] 418-418: Line too long (136/100)
(C0301)
[warning] 419-419: Access to a protected member _format_annotations of a client class
(W0212)
504-506: Add Discord connector to emoji mapping.The Discord connector is missing from the emoji mapping function, which means it will fall back to the default "🔎" emoji.
def get_connector_emoji(connector_name: str) -> str: """Get an appropriate emoji for a connector type.""" connector_emojis = { "YOUTUBE_VIDEO": "📹", "EXTENSION": "🧩", "CRAWLED_URL": "🌐", "FILE": "📄", "SLACK_CONNECTOR": "💬", "NOTION_CONNECTOR": "📘", "GITHUB_CONNECTOR": "🐙", "LINEAR_CONNECTOR": "📊", "TAVILY_API": "🔍", - "LINKUP_API": "🔗" + "LINKUP_API": "🔗", + "DISCORD_CONNECTOR": "🗨️" } return connector_emojis.get(connector_name, "🔎")
520-522: Add Discord connector to friendly name mapping.The Discord connector is missing from the friendly name mapping function, which means it will fall back to the technical name "DISCORD_CONNECTOR".
def get_connector_friendly_name(connector_name: str) -> str: """Convert technical connector IDs to user-friendly names.""" connector_friendly_names = { "YOUTUBE_VIDEO": "YouTube", "EXTENSION": "Browser Extension", "CRAWLED_URL": "Web Pages", "FILE": "Files", "SLACK_CONNECTOR": "Slack", "NOTION_CONNECTOR": "Notion", "GITHUB_CONNECTOR": "GitHub", "LINEAR_CONNECTOR": "Linear", "TAVILY_API": "Tavily Search", - "LINKUP_API": "Linkup Search" + "LINKUP_API": "Linkup Search", + "DISCORD_CONNECTOR": "Discord" } return connector_friendly_names.get(connector_name, connector_name)surfsense_backend/alembic/versions/9_add_discord_connector_enum_and_documenttype.py (2)
10-10: Remove unused import.The
sqlalchemyimport is not used in this migration file.-import sqlalchemy as sa🧰 Tools
🪛 Ruff (0.11.9)
10-10:
sqlalchemyimported but unusedRemove unused import:
sqlalchemy(F401)
🪛 Pylint (3.3.7)
[error] 10-10: Unable to import 'sqlalchemy'
(E0401)
[warning] 10-10: Unused sqlalchemy imported as sa
(W0611)
98-98: Consider breaking long line for better readability.The line exceeds 100 characters. Consider breaking it for better readability.
- f"DELETE FROM {document_table_name} WHERE {document_column_name}::text = '{DOCUMENT_NEW_VALUE}'" + f"DELETE FROM {document_table_name} " + f"WHERE {document_column_name}::text = '{DOCUMENT_NEW_VALUE}'"🧰 Tools
🪛 Pylint (3.3.7)
[convention] 98-98: Line too long (104/100)
(C0301)
surfsense_web/app/dashboard/[search_space_id]/connectors/add/discord-connector/page.tsx (1)
90-95: Improve error handling specificity.The error handling could be more specific about different types of failures to provide better user feedback.
} catch (error) { console.error("Error creating connector:", error); - toast.error(error instanceof Error ? error.message : "Failed to create connector"); + if (error instanceof Error) { + if (error.message.includes("already exists")) { + toast.error("A Discord connector already exists. Each user can have only one Discord connector."); + } else if (error.message.includes("Invalid") || error.message.includes("token")) { + toast.error("Invalid Discord Bot Token. Please check your token and try again."); + } else { + toast.error(error.message); + } + } else { + toast.error("Failed to create connector. Please try again."); + }surfsense_backend/app/utils/connector_service.py (1)
1045-1049: Consider using a constant for the Discord connector ID.The hardcoded ID
11for the Discord connector could be made more maintainable by using a constant or deriving it systematically.Consider defining connector IDs as constants at the module level:
# At the top of the file DISCORD_CONNECTOR_ID = 11 # Then use it in the result object result_object = { "id": DISCORD_CONNECTOR_ID, "name": "Discord", "type": "DISCORD_CONNECTOR", "sources": sources_list, }surfsense_backend/app/routes/search_source_connectors_routes.py (2)
10-10: Fix line length formatting issue.The documentation comment exceeds the 100-character line limit. Consider breaking it into multiple lines for better readability.
-Note: Each user can have only one connector of each type (SERPER_API, TAVILY_API, SLACK_CONNECTOR, NOTION_CONNECTOR, GITHUB_CONNECTOR, LINEAR_CONNECTOR, DISCORD_CONNECTOR). +Note: Each user can have only one connector of each type: +- SERPER_API, TAVILY_API, SLACK_CONNECTOR, NOTION_CONNECTOR +- GITHUB_CONNECTOR, LINEAR_CONNECTOR, DISCORD_CONNECTOR🧰 Tools
🪛 Pylint (3.3.7)
[convention] 10-10: Line too long (172/100)
(C0301)
22-22: Fix import organization and line length.The import statement is too long and should be grouped properly according to Python style guidelines.
-from app.tasks.connectors_indexing_tasks import index_slack_messages, index_notion_pages, index_github_repos, index_linear_issues, index_discord_messages +from app.tasks.connectors_indexing_tasks import ( + index_slack_messages, + index_notion_pages, + index_github_repos, + index_linear_issues, + index_discord_messages +)🧰 Tools
🪛 Pylint (3.3.7)
[convention] 22-22: Line too long (153/100)
(C0301)
[convention] 22-22: Imports from package app are not grouped
(C0412)
surfsense_backend/app/tasks/connectors_indexing_tasks.py (2)
956-956: Use lazy formatting in logging statementsReplace f-strings with lazy formatting in logging calls for better performance. The string formatting will only occur if the log level is enabled.
Example fixes:
-logger.info(f"Starting Discord indexing for connector {connector_id}") +logger.info("Starting Discord indexing for connector %s", connector_id) -logger.info(f"Using last_indexed_at ({start_date.strftime('%Y-%m-%d')}) as start date") +logger.info("Using last_indexed_at (%s) as start date", start_date.strftime('%Y-%m-%d'))Also applies to: 957-957, 968-968, 971-971, 988-988, 990-990, 1004-1004, 1009-1009, 1026-1026, 1080-1080, 1085-1085, 1092-1092, 1096-1096, 1101-1101, 1105-1105
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 956-956: Use lazy % formatting in logging functions
(W1203)
919-1106: Consider extracting document creation logicThe document creation logic (lines 1055-1077) is similar across all indexing functions. Consider extracting this into a shared utility function to promote DRY principles and ensure consistency.
This would reduce code duplication and make it easier to maintain consistent document structure across all connectors.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 947-947: Trailing whitespace
(C0303)
[convention] 950-950: Trailing whitespace
(C0303)
[convention] 955-955: Trailing whitespace
(C0303)
[convention] 961-961: Trailing whitespace
(C0303)
[convention] 964-964: Trailing whitespace
(C0303)
[convention] 971-971: Line too long (122/100)
(C0301)
[convention] 972-972: Trailing whitespace
(C0303)
[convention] 976-976: Trailing whitespace
(C0303)
[convention] 980-980: Trailing whitespace
(C0303)
[convention] 996-996: Trailing whitespace
(C0303)
[convention] 999-999: Trailing whitespace
(C0303)
[convention] 1007-1007: Trailing whitespace
(C0303)
[convention] 1017-1017: Trailing whitespace
(C0303)
[convention] 1032-1032: Trailing whitespace
(C0303)
[convention] 1043-1043: Line too long (101/100)
(C0301)
[convention] 1044-1044: Trailing whitespace
(C0303)
[convention] 1048-1048: Trailing whitespace
(C0303)
[convention] 1049-1049: Line too long (153/100)
(C0301)
[convention] 1050-1050: Line too long (102/100)
(C0301)
[convention] 1052-1052: Line too long (118/100)
(C0301)
[convention] 1075-1075: Trailing whitespace
(C0303)
[convention] 1080-1080: Line too long (102/100)
(C0301)
[convention] 1096-1096: Line too long (113/100)
(C0301)
[convention] 1098-1098: Trailing whitespace
(C0303)
[refactor] 919-919: Too many local variables (35/15)
(R0914)
[warning] 1103-1103: Catching too general exception Exception
(W0718)
[warning] 956-956: Use lazy % formatting in logging functions
(W1203)
[warning] 957-957: Use lazy % formatting in logging functions
(W1203)
[warning] 968-968: Use lazy % formatting in logging functions
(W1203)
[warning] 971-971: Use lazy % formatting in logging functions
(W1203)
[warning] 989-989: Catching too general exception Exception
(W0718)
[warning] 983-983: Access to a protected member _bot_task of a client class
(W0212)
[warning] 984-984: Access to a protected member _wait_until_ready of a client class
(W0212)
[warning] 988-988: Use lazy % formatting in logging functions
(W1203)
[warning] 990-990: Use lazy % formatting in logging functions
(W1203)
[warning] 1004-1004: Use lazy % formatting in logging functions
(W1203)
[warning] 1084-1084: Catching too general exception Exception
(W0718)
[warning] 1009-1009: Use lazy % formatting in logging functions
(W1203)
[warning] 1025-1025: Catching too general exception Exception
(W0718)
[warning] 1026-1026: Use lazy % formatting in logging functions
(W1203)
[warning] 1079-1079: Catching too general exception Exception
(W0718)
[warning] 1080-1080: Use lazy % formatting in logging functions
(W1203)
[warning] 1085-1085: Use lazy % formatting in logging functions
(W1203)
[refactor] 937-1106: Too many nested blocks (7/5)
(R1702)
[warning] 1092-1092: Use lazy % formatting in logging functions
(W1203)
[warning] 1096-1096: Use lazy % formatting in logging functions
(W1203)
[warning] 1101-1101: Use lazy % formatting in logging functions
(W1203)
[warning] 1105-1105: Use lazy % formatting in logging functions
(W1203)
[refactor] 919-919: Too many return statements (7/6)
(R0911)
[refactor] 919-919: Too many branches (19/12)
(R0912)
[refactor] 919-919: Too many statements (93/50)
(R0915)
surfsense_backend/app/connectors/discord_connector.py (4)
40-40: Use lazy formatting in logging statementsUse lazy formatting instead of f-strings in logging calls for better performance.
Example fixes:
-logger.info(f"Logged in as {self.user} (ID: {self.user.id})") +logger.info("Logged in as %s (ID: %s)", self.user, self.user.id) -logger.info(f"Fetched {len(guilds_data)} guilds.") +logger.info("Fetched %d guilds.", len(guilds_data))Also applies to: 107-107, 125-125, 129-129, 139-139, 168-168, 172-172, 175-175, 187-187, 194-194, 208-208, 211-211, 214-214, 235-235, 239-239, 246-246, 254-254, 257-257, 260-260, 263-263
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 40-40: Use lazy % formatting in logging functions
(W1203)
189-194: Improve end date handling for inclusive date rangesThe current implementation adds a hardcoded time of 23:59:59.999999 to the end date, which might miss messages at the very end of the day due to precision issues.
Consider using the start of the next day instead:
if end_date: try: - end_datetime = datetime.datetime.fromisoformat(f"{end_date}T23:59:59.999999").replace(tzinfo=datetime.timezone.utc) + # Use start of next day for inclusive end date + end_date_parsed = datetime.datetime.fromisoformat(end_date).date() + end_datetime = datetime.datetime.combine( + end_date_parsed + datetime.timedelta(days=1), + datetime.time.min + ).replace(tzinfo=datetime.timezone.utc) before = end_datetime except ValueError: logger.warning(f"Invalid end_date format: {end_date}. Ignoring.")🧰 Tools
🪛 Pylint (3.3.7)
[convention] 191-191: Line too long (131/100)
(C0301)
[warning] 194-194: Use lazy % formatting in logging functions
(W1203)
33-33: Remove trailing whitespacesMultiple lines have trailing whitespaces that should be removed.
Clean up trailing whitespaces on lines 33, 54, 69, 78, 138, and 213.
Also applies to: 54-54, 69-69, 78-78, 138-138, 213-213
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 33-33: Line too long (117/100)
(C0301)
21-36: Consider dependency injection for the tokenCurrently, the token can be passed during initialization or set later via
set_token(). This dual approach can lead to confusion about the object's state.Consider requiring the token at initialization to ensure the object is always in a valid state:
- def __init__(self, token: str = None): + def __init__(self, token: str): """ Initialize the DiscordConnector with a bot token. Args: token (str): The Discord bot token. + + Raises: + ValueError: If token is not provided. """ + if not token: + raise ValueError("Discord bot token is required")Then remove the
set_token()method to simplify the API.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 33-33: Line too long (117/100)
(C0301)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
surfsense_backend/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
surfsense_backend/alembic/versions/9_add_discord_connector_enum_and_documenttype.py(1 hunks)surfsense_backend/app/agents/researcher/nodes.py(1 hunks)surfsense_backend/app/agents/researcher/sub_section_writer/prompts.py(1 hunks)surfsense_backend/app/connectors/discord_connector.py(1 hunks)surfsense_backend/app/db.py(2 hunks)surfsense_backend/app/routes/search_source_connectors_routes.py(5 hunks)surfsense_backend/app/schemas/search_source_connector.py(2 hunks)surfsense_backend/app/tasks/connectors_indexing_tasks.py(2 hunks)surfsense_backend/app/utils/connector_service.py(1 hunks)surfsense_backend/pyproject.toml(1 hunks)surfsense_web/app/dashboard/[search_space_id]/connectors/[connector_id]/edit/page.tsx(1 hunks)surfsense_web/app/dashboard/[search_space_id]/connectors/[connector_id]/page.tsx(2 hunks)surfsense_web/app/dashboard/[search_space_id]/connectors/add/discord-connector/page.tsx(1 hunks)surfsense_web/app/dashboard/[search_space_id]/connectors/add/page.tsx(2 hunks)surfsense_web/components/chat/ConnectorComponents.tsx(2 hunks)surfsense_web/components/editConnector/types.ts(1 hunks)surfsense_web/hooks/useConnectorEditPage.ts(4 hunks)surfsense_web/lib/connectors/utils.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
surfsense_web/app/dashboard/[search_space_id]/connectors/[connector_id]/edit/page.tsx (1)
surfsense_web/components/editConnector/EditSimpleTokenForm.tsx (1)
EditSimpleTokenForm(16-37)
surfsense_backend/app/utils/connector_service.py (3)
surfsense_backend/app/agents/researcher/configuration.py (1)
SearchMode(11-14)surfsense_backend/app/retriver/documents_hybrid_search.py (1)
hybrid_search(105-247)surfsense_backend/app/retriver/chunks_hybrid_search.py (1)
hybrid_search(107-243)
surfsense_web/app/dashboard/[search_space_id]/connectors/add/discord-connector/page.tsx (7)
surfsense_web/hooks/useSearchSourceConnectors.ts (1)
useSearchSourceConnectors(24-314)surfsense_web/components/ui/tabs.tsx (4)
Tabs(55-55)TabsList(55-55)TabsTrigger(55-55)TabsContent(55-55)surfsense_web/components/ui/card.tsx (6)
Card(78-78)CardHeader(78-78)CardTitle(78-78)CardDescription(78-78)CardContent(78-78)CardFooter(78-78)surfsense_web/components/ui/alert.tsx (3)
Alert(58-58)AlertTitle(58-58)AlertDescription(58-58)surfsense_web/components/ui/form.tsx (7)
Form(160-160)FormField(166-166)FormItem(161-161)FormLabel(162-162)FormControl(163-163)FormDescription(164-164)FormMessage(165-165)surfsense_web/components/ui/input.tsx (1)
Input(21-21)surfsense_web/components/ui/accordion.tsx (4)
Accordion(60-60)AccordionItem(60-60)AccordionTrigger(60-60)AccordionContent(60-60)
🪛 Pylint (3.3.7)
surfsense_backend/app/schemas/search_source_connector.py
[convention] 84-84: Trailing whitespace
(C0303)
[convention] 94-94: Trailing whitespace
(C0303)
[convention] 99-99: Line too long (126/100)
(C0301)
surfsense_backend/app/agents/researcher/nodes.py
[convention] 403-403: Trailing whitespace
(C0303)
[convention] 418-418: Line too long (136/100)
(C0301)
[warning] 419-419: Access to a protected member _format_annotations of a client class
(W0212)
surfsense_backend/app/utils/connector_service.py
[convention] 962-962: Trailing whitespace
(C0303)
[convention] 963-963: Line too long (159/100)
(C0301)
[convention] 994-994: Trailing whitespace
(C0303)
[convention] 1018-1018: Trailing whitespace
(C0303)
[convention] 1023-1023: Trailing whitespace
(C0303)
[convention] 1028-1028: Trailing whitespace
(C0303)
[convention] 1029-1029: Line too long (106/100)
(C0301)
[convention] 1043-1043: Trailing whitespace
(C0303)
[convention] 1051-1051: Trailing whitespace
(C0303)
[convention] 1054-1054: Trailing newlines
(C0305)
[refactor] 963-963: Too many arguments (6/5)
(R0913)
[refactor] 963-963: Too many positional arguments (6/5)
(R0917)
[refactor] 963-963: Too many local variables (20/15)
(R0914)
surfsense_backend/app/routes/search_source_connectors_routes.py
[convention] 10-10: Line too long (172/100)
(C0301)
[convention] 22-22: Line too long (153/100)
(C0301)
[convention] 22-22: Imports from package app are not grouped
(C0412)
[convention] 393-393: Trailing whitespace
(C0303)
[convention] 399-399: Line too long (111/100)
(C0301)
[warning] 398-400: Use lazy % formatting in logging functions
(W1203)
[convention] 642-642: Line too long (110/100)
(C0301)
[convention] 646-646: Final newline missing
(C0304)
[warning] 645-645: Catching too general exception Exception
(W0718)
[warning] 642-642: Use lazy % formatting in logging functions
(W1203)
[warning] 644-644: Use lazy % formatting in logging functions
(W1203)
[warning] 646-646: Use lazy % formatting in logging functions
(W1203)
surfsense_backend/alembic/versions/9_add_discord_connector_enum_and_documenttype.py
[convention] 98-98: Line too long (104/100)
(C0301)
[convention] 1-1: Module name "9_add_discord_connector_enum_and_documenttype" doesn't conform to snake_case naming style
(C0103)
[error] 9-9: Unable to import 'alembic'
(E0401)
[error] 10-10: Unable to import 'sqlalchemy'
(E0401)
[warning] 10-10: Unused sqlalchemy imported as sa
(W0611)
surfsense_backend/app/tasks/connectors_indexing_tasks.py
[error] 15-15: Unable to import 'discord'
(E0401)
[error] 16-16: Unable to import 'slack_sdk.errors'
(E0401)
[convention] 17-17: standard import "logging" should be placed before third party imports "sqlalchemy.ext.asyncio.AsyncSession", "sqlalchemy.exc.SQLAlchemyError", "sqlalchemy.future.select" (...) "app.connectors.discord_connector.DiscordConnector", "discord.DiscordException", "slack_sdk.errors.SlackApiError"
(C0411)
[convention] 18-18: standard import "asyncio" should be placed before third party imports "sqlalchemy.ext.asyncio.AsyncSession", "sqlalchemy.exc.SQLAlchemyError", "sqlalchemy.future.select" (...) "app.connectors.discord_connector.DiscordConnector", "discord.DiscordException", "slack_sdk.errors.SlackApiError"
(C0411)
[warning] 15-15: Unused DiscordException imported from discord
(W0611)
[convention] 947-947: Trailing whitespace
(C0303)
[convention] 950-950: Trailing whitespace
(C0303)
[convention] 955-955: Trailing whitespace
(C0303)
[convention] 961-961: Trailing whitespace
(C0303)
[convention] 964-964: Trailing whitespace
(C0303)
[convention] 971-971: Line too long (122/100)
(C0301)
[convention] 972-972: Trailing whitespace
(C0303)
[convention] 976-976: Trailing whitespace
(C0303)
[convention] 980-980: Trailing whitespace
(C0303)
[convention] 996-996: Trailing whitespace
(C0303)
[convention] 999-999: Trailing whitespace
(C0303)
[convention] 1007-1007: Trailing whitespace
(C0303)
[convention] 1017-1017: Trailing whitespace
(C0303)
[convention] 1032-1032: Trailing whitespace
(C0303)
[convention] 1043-1043: Line too long (101/100)
(C0301)
[convention] 1044-1044: Trailing whitespace
(C0303)
[convention] 1048-1048: Trailing whitespace
(C0303)
[convention] 1049-1049: Line too long (153/100)
(C0301)
[convention] 1050-1050: Line too long (102/100)
(C0301)
[convention] 1052-1052: Line too long (118/100)
(C0301)
[convention] 1075-1075: Trailing whitespace
(C0303)
[convention] 1080-1080: Line too long (102/100)
(C0301)
[convention] 1096-1096: Line too long (113/100)
(C0301)
[convention] 1098-1098: Trailing whitespace
(C0303)
[refactor] 919-919: Too many local variables (35/15)
(R0914)
[warning] 1103-1103: Catching too general exception Exception
(W0718)
[warning] 956-956: Use lazy % formatting in logging functions
(W1203)
[warning] 957-957: Use lazy % formatting in logging functions
(W1203)
[warning] 968-968: Use lazy % formatting in logging functions
(W1203)
[warning] 971-971: Use lazy % formatting in logging functions
(W1203)
[warning] 989-989: Catching too general exception Exception
(W0718)
[warning] 983-983: Access to a protected member _bot_task of a client class
(W0212)
[warning] 984-984: Access to a protected member _wait_until_ready of a client class
(W0212)
[warning] 988-988: Use lazy % formatting in logging functions
(W1203)
[warning] 990-990: Use lazy % formatting in logging functions
(W1203)
[warning] 1004-1004: Use lazy % formatting in logging functions
(W1203)
[warning] 1084-1084: Catching too general exception Exception
(W0718)
[warning] 1009-1009: Use lazy % formatting in logging functions
(W1203)
[warning] 1025-1025: Catching too general exception Exception
(W0718)
[warning] 1026-1026: Use lazy % formatting in logging functions
(W1203)
[warning] 1079-1079: Catching too general exception Exception
(W0718)
[warning] 1080-1080: Use lazy % formatting in logging functions
(W1203)
[warning] 1085-1085: Use lazy % formatting in logging functions
(W1203)
[refactor] 937-1106: Too many nested blocks (7/5)
(R1702)
[warning] 1092-1092: Use lazy % formatting in logging functions
(W1203)
[warning] 1096-1096: Use lazy % formatting in logging functions
(W1203)
[warning] 1101-1101: Use lazy % formatting in logging functions
(W1203)
[warning] 1105-1105: Use lazy % formatting in logging functions
(W1203)
[refactor] 919-919: Too many return statements (7/6)
(R0911)
[refactor] 919-919: Too many branches (19/12)
(R0912)
[refactor] 919-919: Too many statements (93/50)
(R0915)
surfsense_backend/app/connectors/discord_connector.py
[convention] 33-33: Line too long (117/100)
(C0301)
[convention] 54-54: Trailing whitespace
(C0303)
[convention] 69-69: Trailing whitespace
(C0303)
[convention] 78-78: Trailing whitespace
(C0303)
[convention] 138-138: Trailing whitespace
(C0303)
[convention] 184-184: Line too long (114/100)
(C0301)
[convention] 191-191: Line too long (131/100)
(C0301)
[convention] 208-208: Line too long (107/100)
(C0301)
[convention] 213-213: Trailing whitespace
(C0303)
[convention] 260-260: Line too long (136/100)
(C0301)
[error] 10-10: Unable to import 'discord'
(E0401)
[error] 11-11: Unable to import 'discord.ext'
(E0401)
[warning] 40-40: Use lazy % formatting in logging functions
(W1203)
[warning] 107-107: Use lazy % formatting in logging functions
(W1203)
[warning] 125-125: Use lazy % formatting in logging functions
(W1203)
[warning] 129-129: Use lazy % formatting in logging functions
(W1203)
[warning] 139-139: Use lazy % formatting in logging functions
(W1203)
[warning] 168-168: Use lazy % formatting in logging functions
(W1203)
[warning] 172-172: Use lazy % formatting in logging functions
(W1203)
[warning] 175-175: Use lazy % formatting in logging functions
(W1203)
[warning] 187-187: Use lazy % formatting in logging functions
(W1203)
[warning] 194-194: Use lazy % formatting in logging functions
(W1203)
[warning] 208-208: Use lazy % formatting in logging functions
(W1203)
[warning] 211-211: Use lazy % formatting in logging functions
(W1203)
[warning] 214-214: Use lazy % formatting in logging functions
(W1203)
[warning] 235-235: Use lazy % formatting in logging functions
(W1203)
[warning] 239-239: Use lazy % formatting in logging functions
(W1203)
[warning] 246-246: Use lazy % formatting in logging functions
(W1203)
[warning] 254-254: Use lazy % formatting in logging functions
(W1203)
[warning] 257-257: Use lazy % formatting in logging functions
(W1203)
[warning] 260-260: Use lazy % formatting in logging functions
(W1203)
[warning] 263-263: Use lazy % formatting in logging functions
(W1203)
[convention] 12-12: standard import "datetime" should be placed before third party imports "discord", "discord.ext.commands"
(C0411)
[convention] 13-13: standard import "asyncio" should be placed before third party imports "discord", "discord.ext.commands"
(C0411)
🪛 Ruff (0.11.9)
surfsense_backend/alembic/versions/9_add_discord_connector_enum_and_documenttype.py
10-10: sqlalchemy imported but unused
Remove unused import: sqlalchemy
(F401)
surfsense_backend/app/tasks/connectors_indexing_tasks.py
15-15: discord.DiscordException imported but unused
Remove unused import: discord.DiscordException
(F401)
🔇 Additional comments (21)
surfsense_web/lib/connectors/utils.ts (1)
10-10: LGTM! Consistent with existing pattern.The Discord connector mapping follows the same pattern as other connectors and maintains consistency with the display naming convention.
surfsense_backend/app/agents/researcher/sub_section_writer/prompts.py (1)
18-18: LGTM! Accurate description and consistent formatting.The Discord connector addition accurately describes its purpose as "personal community interactions" and follows the same format as other knowledge sources in the citation system.
surfsense_web/components/chat/ConnectorComponents.tsx (2)
14-14: LGTM! Proper icon import following established pattern.The IconBrandDiscord import is correctly added to the existing import statement from @tabler/icons-react, maintaining consistency with other brand icons.
44-45: LGTM! Consistent icon implementation.The Discord connector case follows the same pattern as other brand connectors (Slack, Notion, GitHub) and correctly returns the Discord icon with appropriate props.
surfsense_backend/app/db.py (1)
53-53: LGTM! Consistent enum additions for Discord connector support.The additions to both
DocumentTypeandSearchSourceConnectorTypeenums follow the established naming conventions and patterns used by other connector types in the codebase.Also applies to: 63-63
surfsense_web/app/dashboard/[search_space_id]/connectors/[connector_id]/page.tsx (2)
55-55: Proper display name mapping for Discord connector.The display name mapping follows the established pattern and provides a user-friendly name for the Discord connector type.
93-93: Correct API key field mapping for Discord bot token.The field name "DISCORD_BOT_TOKEN" appropriately identifies the Discord-specific token field and follows Discord's standard token naming convention.
surfsense_web/app/dashboard/[search_space_id]/connectors/add/page.tsx (2)
82-82: Good improvements to Discord connector configuration.The ID change to "discord-connector" improves consistency with other connector naming patterns, and changing the status to "available" correctly reflects that the Discord connector is now functional.
Also applies to: 86-86
193-193: Appropriate UX improvement for team-chats visibility.Adding "team-chats" to the default expanded categories ensures users can immediately see the newly available Discord connector without needing to expand the category manually.
surfsense_backend/app/schemas/search_source_connector.py (1)
95-103: LGTM! Discord connector validation follows established patterns.The validation logic for the Discord connector is consistent with other connector types, properly validating the required
DISCORD_BOT_TOKENfield and ensuring it's not empty.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 99-99: Line too long (126/100)
(C0301)
surfsense_backend/app/agents/researcher/nodes.py (1)
404-419: LGTM! Discord connector integration follows established patterns.The Discord connector implementation correctly follows the same pattern as other connectors:
- Calls the appropriate service method
- Handles source and document collection
- Provides streaming feedback with appropriate emoji and messaging
The code maintains consistency with the existing connector architecture.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 418-418: Line too long (136/100)
(C0301)
[warning] 419-419: Access to a protected member _format_annotations of a client class
(W0212)
surfsense_web/hooks/useConnectorEditPage.ts (4)
44-46: LGTM! Discord bot token added to form defaults.The Discord bot token field is correctly added to the form's default values, following the same pattern as other connector tokens.
64-66: LGTM! Discord bot token added to form reset logic.The Discord bot token is correctly included in the form reset logic when loading connector data, maintaining consistency with other connector tokens.
176-181: LGTM! Discord connector save logic follows established patterns.The save handler for Discord connector correctly:
- Compares the new token with the original configuration
- Validates that the token is not empty before saving
- Sets up the new configuration object properly
This follows the exact same pattern as other token-based connectors like Slack and Notion.
223-225: LGTM! Discord bot token form value update is correctly implemented.The form value update for Discord bot token after successful save is properly implemented, following the same pattern as other connector tokens to ensure form state consistency.
surfsense_backend/alembic/versions/9_add_discord_connector_enum_and_documenttype.py (2)
26-31: LGTM! Upgrade function correctly adds enum values.The upgrade function properly adds the
DISCORD_CONNECTORvalue to both thesearchsourceconnectortypeanddocumenttypeenums using the standard PostgreSQLALTER TYPE ... ADD VALUEsyntax.
34-111: Excellent downgrade implementation with proper data cleanup.The downgrade function follows best practices for enum modifications in PostgreSQL:
- Properly recreates enums without the new value
- Handles data cleanup by deleting documents with the Discord connector type
- Uses proper casting when updating column types
- Cleans up temporary enum types
This ensures a clean rollback path without data integrity issues.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 98-98: Line too long (104/100)
(C0301)
surfsense_web/app/dashboard/[search_space_id]/connectors/add/discord-connector/page.tsx (1)
78-86: LGTM! Well-structured connector creation.The connector creation logic properly maps the form values to the expected API format with appropriate configuration structure. The
is_indexable: trueandlast_indexed_at: nulldefaults are correct for new connectors.surfsense_backend/app/utils/connector_service.py (2)
963-975: LGTM! Consistent method signature and documentation.The method follows the established pattern from other connector search methods with proper type hints, documentation, and parameter handling. The docstring clearly explains the purpose and return format.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 963-963: Line too long (159/100)
(C0301)
[refactor] 963-963: Too many arguments (6/5)
(R0913)
[refactor] 963-963: Too many positional arguments (6/5)
(R0917)
[refactor] 963-963: Too many local variables (20/15)
(R0914)
976-994: LGTM! Consistent hybrid search implementation.The hybrid search logic properly handles both CHUNKS and DOCUMENTS modes, following the same pattern as other connector search methods. The document results transformation is correctly applied for DOCUMENTS mode.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 994-994: Trailing whitespace
(C0303)
surfsense_backend/app/routes/search_source_connectors_routes.py (1)
382-404: LGTM! Consistent Discord indexing logic.The Discord connector indexing implementation follows the established pattern from other connectors with proper date range calculation, background task scheduling, and logging. The logic correctly handles incremental indexing based on the last indexed timestamp.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 393-393: Trailing whitespace
(C0303)
[convention] 399-399: Line too long (111/100)
(C0301)
[warning] 398-400: Use lazy % formatting in logging functions
(W1203)
surfsense_web/app/dashboard/[search_space_id]/connectors/add/discord-connector/page.tsx
Outdated
Show resolved
Hide resolved
surfsense_web/app/dashboard/[search_space_id]/connectors/add/discord-connector/page.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
surfsense_backend/app/tasks/connectors_indexing_tasks.py (1)
919-1106: 🛠️ Refactor suggestionConsider comprehensive refactoring to reduce complexity.
This function has significant complexity issues beyond those previously identified:
- 35 local variables (exceeds recommended 15)
- 7 levels of nesting (exceeds recommended 5)
- 93 statements (exceeds recommended 50)
- 19 branches (exceeds recommended 12)
The function attempts to handle guild fetching, channel processing, message retrieval, and document creation all in one place, making it difficult to test and maintain.
Consider extracting these into separate helper functions:
_process_guild_messages(guild, discord_client, session, search_space_id, start_date_str, end_date_str)_process_channel_messages(channel, guild_name, discord_client, session, search_space_id, start_date_str, end_date_str)_create_discord_document(message, guild_info, channel_info, session, search_space_id)This would improve readability, enable better error handling per component, and make unit testing much easier.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 947-947: Trailing whitespace
(C0303)
[convention] 950-950: Trailing whitespace
(C0303)
[convention] 955-955: Trailing whitespace
(C0303)
[convention] 961-961: Trailing whitespace
(C0303)
[convention] 964-964: Trailing whitespace
(C0303)
[convention] 971-971: Line too long (122/100)
(C0301)
[convention] 972-972: Trailing whitespace
(C0303)
[convention] 976-976: Trailing whitespace
(C0303)
[convention] 980-980: Trailing whitespace
(C0303)
[convention] 999-999: Trailing whitespace
(C0303)
[convention] 1007-1007: Trailing whitespace
(C0303)
[convention] 1017-1017: Trailing whitespace
(C0303)
[convention] 1031-1031: Trailing whitespace
(C0303)
[convention] 1042-1042: Line too long (101/100)
(C0301)
[convention] 1043-1043: Trailing whitespace
(C0303)
[convention] 1047-1047: Trailing whitespace
(C0303)
[convention] 1048-1048: Line too long (153/100)
(C0301)
[convention] 1049-1049: Line too long (102/100)
(C0301)
[convention] 1051-1051: Line too long (118/100)
(C0301)
[convention] 1074-1074: Trailing whitespace
(C0303)
[convention] 1079-1079: Line too long (102/100)
(C0301)
[convention] 1095-1095: Line too long (113/100)
(C0301)
[convention] 1097-1097: Trailing whitespace
(C0303)
[refactor] 919-919: Too many local variables (35/15)
(R0914)
[warning] 1102-1102: Catching too general exception Exception
(W0718)
[warning] 956-956: Use lazy % formatting in logging functions
(W1203)
[warning] 957-957: Use lazy % formatting in logging functions
(W1203)
[warning] 968-968: Use lazy % formatting in logging functions
(W1203)
[warning] 971-971: Use lazy % formatting in logging functions
(W1203)
[warning] 989-989: Catching too general exception Exception
(W0718)
[warning] 983-983: Access to a protected member _bot_task of a client class
(W0212)
[warning] 984-984: Access to a protected member _wait_until_ready of a client class
(W0212)
[warning] 988-988: Use lazy % formatting in logging functions
(W1203)
[warning] 990-990: Use lazy % formatting in logging functions
(W1203)
[warning] 1004-1004: Use lazy % formatting in logging functions
(W1203)
[warning] 1083-1083: Catching too general exception Exception
(W0718)
[warning] 1009-1009: Use lazy % formatting in logging functions
(W1203)
[warning] 1024-1024: Catching too general exception Exception
(W0718)
[warning] 1025-1025: Use lazy % formatting in logging functions
(W1203)
[warning] 1078-1078: Catching too general exception Exception
(W0718)
[warning] 1079-1079: Use lazy % formatting in logging functions
(W1203)
[warning] 1084-1084: Use lazy % formatting in logging functions
(W1203)
[refactor] 937-1105: Too many nested blocks (7/5)
(R1702)
[warning] 1091-1091: Use lazy % formatting in logging functions
(W1203)
[warning] 1095-1095: Use lazy % formatting in logging functions
(W1203)
[warning] 1100-1100: Use lazy % formatting in logging functions
(W1203)
[warning] 1104-1104: Use lazy % formatting in logging functions
(W1203)
[refactor] 919-919: Too many return statements (7/6)
(R0911)
[refactor] 919-919: Too many branches (19/12)
(R0912)
[refactor] 919-919: Too many statements (93/50)
(R0915)
♻️ Duplicate comments (6)
surfsense_backend/app/tasks/connectors_indexing_tasks.py (4)
15-15: Remove unused import as previously identified.The
DiscordExceptionimport remains unused and should be removed as indicated in previous reviews.🧰 Tools
🪛 Ruff (0.11.9)
15-15:
discord.DiscordExceptionimported but unusedRemove unused import:
discord.DiscordException(F401)
🪛 Pylint (3.3.7)
[error] 15-15: Unable to import 'discord'
(E0401)
[warning] 15-15: Unused DiscordException imported from discord
(W0611)
956-957: Security issue with token logging - duplicate of previous feedback.Logging any part of sensitive credentials like Discord tokens poses a security risk as previously identified.
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 956-956: Use lazy % formatting in logging functions
(W1203)
[warning] 957-957: Use lazy % formatting in logging functions
(W1203)
983-984: Protected member access violation - duplicate of previous feedback.The direct access to
_bot_taskand_wait_until_readyprotected members violates encapsulation as previously noted.🧰 Tools
🪛 Pylint (3.3.7)
[warning] 983-983: Access to a protected member _bot_task of a client class
(W0212)
[warning] 984-984: Access to a protected member _wait_until_ready of a client class
(W0212)
1048-1049: Inconsistent summary generation approach - duplicate of previous feedback.Discord connector uses manual string concatenation for summaries instead of LLM like other connectors, as previously identified.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1048-1048: Line too long (153/100)
(C0301)
[convention] 1049-1049: Line too long (102/100)
(C0301)
surfsense_backend/app/connectors/discord_connector.py (2)
10-13: Import ordering issue - duplicate of previous feedback.Standard library imports should be placed before third-party imports as previously identified.
🧰 Tools
🪛 Pylint (3.3.7)
[error] 10-10: Unable to import 'discord'
(E0401)
[error] 11-11: Unable to import 'discord.ext'
(E0401)
[convention] 12-12: standard import "datetime" should be placed before third party imports "discord", "discord.ext.commands"
(C0411)
[convention] 13-13: standard import "asyncio" should be placed before third party imports "discord", "discord.ext.commands"
(C0411)
118-118: Sleep workaround should be replaced - duplicate of previous feedback.The
asyncio.sleep(1)workaround is unreliable and should be replaced with proper synchronization as previously noted.
🧹 Nitpick comments (1)
surfsense_backend/app/connectors/discord_connector.py (1)
124-124: Remove unnecessary f-string prefix.The f-string on line 124 doesn't contain any placeholders, making the
fprefix unnecessary.- logger.error(f"Bot did not become ready within 60 seconds. Connection may have failed.") + logger.error("Bot did not become ready within 60 seconds. Connection may have failed.")🧰 Tools
🪛 Ruff (0.11.9)
124-124: f-string without any placeholders
Remove extraneous
fprefix(F541)
🪛 Pylint (3.3.7)
[warning] 124-124: Use lazy % formatting in logging functions
(W1203)
[warning] 124-124: Using an f-string that does not have any interpolated variables
(W1309)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
surfsense_backend/app/connectors/discord_connector.py(1 hunks)surfsense_backend/app/tasks/connectors_indexing_tasks.py(2 hunks)surfsense_web/app/dashboard/[search_space_id]/documents/(manage)/page.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- surfsense_web/app/dashboard/[search_space_id]/documents/(manage)/page.tsx
🧰 Additional context used
🪛 Ruff (0.11.9)
surfsense_backend/app/connectors/discord_connector.py
124-124: f-string without any placeholders
Remove extraneous f prefix
(F541)
surfsense_backend/app/tasks/connectors_indexing_tasks.py
15-15: discord.DiscordException imported but unused
Remove unused import: discord.DiscordException
(F401)
🪛 Pylint (3.3.7)
surfsense_backend/app/connectors/discord_connector.py
[convention] 33-33: Line too long (117/100)
(C0301)
[convention] 64-64: Trailing whitespace
(C0303)
[convention] 67-67: Line too long (102/100)
(C0301)
[convention] 76-76: Line too long (137/100)
(C0301)
[convention] 110-110: Trailing whitespace
(C0303)
[convention] 119-119: Trailing whitespace
(C0303)
[convention] 186-186: Trailing whitespace
(C0303)
[convention] 230-230: Line too long (114/100)
(C0301)
[convention] 237-237: Line too long (115/100)
(C0301)
[convention] 254-254: Line too long (107/100)
(C0301)
[convention] 259-259: Trailing whitespace
(C0303)
[convention] 306-306: Line too long (136/100)
(C0301)
[error] 10-10: Unable to import 'discord'
(E0401)
[error] 11-11: Unable to import 'discord.ext'
(E0401)
[warning] 41-41: Use lazy % formatting in logging functions
(W1203)
[warning] 76-76: Use lazy % formatting in logging functions
(W1203)
[warning] 80-80: Use lazy % formatting in logging functions
(W1203)
[warning] 84-84: Use lazy % formatting in logging functions
(W1203)
[warning] 124-124: Use lazy % formatting in logging functions
(W1203)
[warning] 124-124: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 127-127: Use lazy % formatting in logging functions
(W1203)
[warning] 155-155: Use lazy % formatting in logging functions
(W1203)
[warning] 173-173: Use lazy % formatting in logging functions
(W1203)
[warning] 177-177: Use lazy % formatting in logging functions
(W1203)
[warning] 187-187: Use lazy % formatting in logging functions
(W1203)
[warning] 214-214: Use lazy % formatting in logging functions
(W1203)
[warning] 218-218: Use lazy % formatting in logging functions
(W1203)
[warning] 221-221: Use lazy % formatting in logging functions
(W1203)
[warning] 233-233: Use lazy % formatting in logging functions
(W1203)
[warning] 240-240: Use lazy % formatting in logging functions
(W1203)
[warning] 254-254: Use lazy % formatting in logging functions
(W1203)
[warning] 257-257: Use lazy % formatting in logging functions
(W1203)
[warning] 260-260: Use lazy % formatting in logging functions
(W1203)
[warning] 281-281: Use lazy % formatting in logging functions
(W1203)
[warning] 285-285: Use lazy % formatting in logging functions
(W1203)
[warning] 292-292: Use lazy % formatting in logging functions
(W1203)
[warning] 300-300: Use lazy % formatting in logging functions
(W1203)
[warning] 303-303: Use lazy % formatting in logging functions
(W1203)
[warning] 306-306: Use lazy % formatting in logging functions
(W1203)
[warning] 309-309: Use lazy % formatting in logging functions
(W1203)
[convention] 12-12: standard import "datetime" should be placed before third party imports "discord", "discord.ext.commands"
(C0411)
[convention] 13-13: standard import "asyncio" should be placed before third party imports "discord", "discord.ext.commands"
(C0411)
surfsense_backend/app/tasks/connectors_indexing_tasks.py
[error] 15-15: Unable to import 'discord'
(E0401)
[error] 16-16: Unable to import 'slack_sdk.errors'
(E0401)
[convention] 17-17: standard import "logging" should be placed before third party imports "sqlalchemy.ext.asyncio.AsyncSession", "sqlalchemy.exc.SQLAlchemyError", "sqlalchemy.future.select" (...) "app.connectors.discord_connector.DiscordConnector", "discord.DiscordException", "slack_sdk.errors.SlackApiError"
(C0411)
[convention] 18-18: standard import "asyncio" should be placed before third party imports "sqlalchemy.ext.asyncio.AsyncSession", "sqlalchemy.exc.SQLAlchemyError", "sqlalchemy.future.select" (...) "app.connectors.discord_connector.DiscordConnector", "discord.DiscordException", "slack_sdk.errors.SlackApiError"
(C0411)
[warning] 15-15: Unused DiscordException imported from discord
(W0611)
[convention] 947-947: Trailing whitespace
(C0303)
[convention] 950-950: Trailing whitespace
(C0303)
[convention] 955-955: Trailing whitespace
(C0303)
[convention] 961-961: Trailing whitespace
(C0303)
[convention] 964-964: Trailing whitespace
(C0303)
[convention] 971-971: Line too long (122/100)
(C0301)
[convention] 972-972: Trailing whitespace
(C0303)
[convention] 976-976: Trailing whitespace
(C0303)
[convention] 980-980: Trailing whitespace
(C0303)
[convention] 999-999: Trailing whitespace
(C0303)
[convention] 1007-1007: Trailing whitespace
(C0303)
[convention] 1017-1017: Trailing whitespace
(C0303)
[convention] 1031-1031: Trailing whitespace
(C0303)
[convention] 1042-1042: Line too long (101/100)
(C0301)
[convention] 1043-1043: Trailing whitespace
(C0303)
[convention] 1047-1047: Trailing whitespace
(C0303)
[convention] 1048-1048: Line too long (153/100)
(C0301)
[convention] 1049-1049: Line too long (102/100)
(C0301)
[convention] 1051-1051: Line too long (118/100)
(C0301)
[convention] 1074-1074: Trailing whitespace
(C0303)
[convention] 1079-1079: Line too long (102/100)
(C0301)
[convention] 1095-1095: Line too long (113/100)
(C0301)
[convention] 1097-1097: Trailing whitespace
(C0303)
[refactor] 919-919: Too many local variables (35/15)
(R0914)
[warning] 1102-1102: Catching too general exception Exception
(W0718)
[warning] 956-956: Use lazy % formatting in logging functions
(W1203)
[warning] 957-957: Use lazy % formatting in logging functions
(W1203)
[warning] 968-968: Use lazy % formatting in logging functions
(W1203)
[warning] 971-971: Use lazy % formatting in logging functions
(W1203)
[warning] 989-989: Catching too general exception Exception
(W0718)
[warning] 983-983: Access to a protected member _bot_task of a client class
(W0212)
[warning] 984-984: Access to a protected member _wait_until_ready of a client class
(W0212)
[warning] 988-988: Use lazy % formatting in logging functions
(W1203)
[warning] 990-990: Use lazy % formatting in logging functions
(W1203)
[warning] 1004-1004: Use lazy % formatting in logging functions
(W1203)
[warning] 1083-1083: Catching too general exception Exception
(W0718)
[warning] 1009-1009: Use lazy % formatting in logging functions
(W1203)
[warning] 1024-1024: Catching too general exception Exception
(W0718)
[warning] 1025-1025: Use lazy % formatting in logging functions
(W1203)
[warning] 1078-1078: Catching too general exception Exception
(W0718)
[warning] 1079-1079: Use lazy % formatting in logging functions
(W1203)
[warning] 1084-1084: Use lazy % formatting in logging functions
(W1203)
[refactor] 937-1105: Too many nested blocks (7/5)
(R1702)
[warning] 1091-1091: Use lazy % formatting in logging functions
(W1203)
[warning] 1095-1095: Use lazy % formatting in logging functions
(W1203)
[warning] 1100-1100: Use lazy % formatting in logging functions
(W1203)
[warning] 1104-1104: Use lazy % formatting in logging functions
(W1203)
[refactor] 919-919: Too many return statements (7/6)
(R0911)
[refactor] 919-919: Too many branches (19/12)
(R0912)
[refactor] 919-919: Too many statements (93/50)
(R0915)
🔇 Additional comments (1)
surfsense_backend/app/connectors/discord_connector.py (1)
18-311: Well-structured Discord connector implementation.The overall design of the
DiscordConnectorclass is solid:✅ Properly extends
commands.Botwith appropriate intents
✅ Comprehensive error handling with specific exception types
✅ Good logging coverage for debugging
✅ Clear method documentation
✅ Appropriate async/await usage throughoutThe API design provides a clean interface for guild, channel, and message operations that integrates well with the indexing system.
🧰 Tools
🪛 Ruff (0.11.9)
124-124: f-string without any placeholders
Remove extraneous
fprefix(F541)
🪛 Pylint (3.3.7)
[convention] 33-33: Line too long (117/100)
(C0301)
[convention] 64-64: Trailing whitespace
(C0303)
[convention] 67-67: Line too long (102/100)
(C0301)
[convention] 76-76: Line too long (137/100)
(C0301)
[convention] 110-110: Trailing whitespace
(C0303)
[convention] 119-119: Trailing whitespace
(C0303)
[convention] 186-186: Trailing whitespace
(C0303)
[convention] 230-230: Line too long (114/100)
(C0301)
[convention] 237-237: Line too long (115/100)
(C0301)
[convention] 254-254: Line too long (107/100)
(C0301)
[convention] 259-259: Trailing whitespace
(C0303)
[convention] 306-306: Line too long (136/100)
(C0301)
[warning] 41-41: Use lazy % formatting in logging functions
(W1203)
[warning] 76-76: Use lazy % formatting in logging functions
(W1203)
[warning] 80-80: Use lazy % formatting in logging functions
(W1203)
[warning] 84-84: Use lazy % formatting in logging functions
(W1203)
[warning] 124-124: Use lazy % formatting in logging functions
(W1203)
[warning] 124-124: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 127-127: Use lazy % formatting in logging functions
(W1203)
[warning] 155-155: Use lazy % formatting in logging functions
(W1203)
[warning] 173-173: Use lazy % formatting in logging functions
(W1203)
[warning] 177-177: Use lazy % formatting in logging functions
(W1203)
[warning] 187-187: Use lazy % formatting in logging functions
(W1203)
[warning] 214-214: Use lazy % formatting in logging functions
(W1203)
[warning] 218-218: Use lazy % formatting in logging functions
(W1203)
[warning] 221-221: Use lazy % formatting in logging functions
(W1203)
[warning] 233-233: Use lazy % formatting in logging functions
(W1203)
[warning] 240-240: Use lazy % formatting in logging functions
(W1203)
[warning] 254-254: Use lazy % formatting in logging functions
(W1203)
[warning] 257-257: Use lazy % formatting in logging functions
(W1203)
[warning] 260-260: Use lazy % formatting in logging functions
(W1203)
[warning] 281-281: Use lazy % formatting in logging functions
(W1203)
[warning] 285-285: Use lazy % formatting in logging functions
(W1203)
[warning] 292-292: Use lazy % formatting in logging functions
(W1203)
[warning] 300-300: Use lazy % formatting in logging functions
(W1203)
[warning] 303-303: Use lazy % formatting in logging functions
(W1203)
[warning] 306-306: Use lazy % formatting in logging functions
(W1203)
[warning] 309-309: Use lazy % formatting in logging functions
(W1203)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
surfsense_backend/app/tasks/connectors_indexing_tasks.py (3)
14-17: Remove unused import to clean up dependencies.The import statement appears to include unnecessary dependencies.
Based on static analysis, the
DiscordExceptionimport from line 15 is unused. Please remove it:from app.connectors.discord_connector import DiscordConnector -from discord import DiscordException🧰 Tools
🪛 Pylint (3.3.7)
[error] 15-15: Unable to import 'slack_sdk.errors'
(E0401)
[convention] 16-16: standard import "logging" should be placed before third party imports "sqlalchemy.ext.asyncio.AsyncSession", "sqlalchemy.exc.SQLAlchemyError", "sqlalchemy.future.select" (...) "app.connectors.linear_connector.LinearConnector", "app.connectors.discord_connector.DiscordConnector", "slack_sdk.errors.SlackApiError"
(C0411)
[convention] 17-17: standard import "asyncio" should be placed before third party imports "sqlalchemy.ext.asyncio.AsyncSession", "sqlalchemy.exc.SQLAlchemyError", "sqlalchemy.future.select" (...) "app.connectors.linear_connector.LinearConnector", "app.connectors.discord_connector.DiscordConnector", "slack_sdk.errors.SlackApiError"
(C0411)
955-955:⚠️ Potential issueSecurity: Avoid logging sensitive credentials.
Logging any part of API tokens poses a security risk as they could be exposed in log files or monitoring systems.
Remove any logging of the Discord token entirely:
- logger.info(f"discord token is: {discord_token[:4]}... (truncated for security)")Only log non-sensitive information like connector IDs or general status messages.
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 955-955: Use lazy % formatting in logging functions
(W1203)
981-982: 🛠️ Refactor suggestionAvoid accessing protected members of external classes.
Direct access to protected members violates encapsulation principles and could break if the Discord library changes its internal implementation.
Consider refactoring the
DiscordConnectorclass to expose proper public methods for bot lifecycle management:- discord_client._bot_task = asyncio.create_task(discord_client.start_bot()) - await discord_client._wait_until_ready() + await discord_client.start_and_wait_ready()Would you like me to help design public methods in the
DiscordConnectorclass for proper bot lifecycle management?🧰 Tools
🪛 Pylint (3.3.7)
[warning] 981-981: Access to a protected member _bot_task of a client class
(W0212)
[warning] 982-982: Access to a protected member _wait_until_ready of a client class
(W0212)
🧹 Nitpick comments (2)
surfsense_backend/app/utils/connector_service.py (1)
962-1055: LGTM! Discord search method implementation looks good.The Discord search method correctly follows the established pattern of other connector search methods. The URL construction has been properly implemented to handle both guild channels and DM channels with appropriate fallbacks.
However, there are some minor formatting issues to address:
Apply this diff to fix trailing whitespace and improve formatting:
- + - + - + - + - + - +🧰 Tools
🪛 Pylint (3.3.7)
[convention] 962-962: Trailing whitespace
(C0303)
[convention] 963-963: Line too long (159/100)
(C0301)
[convention] 994-994: Trailing whitespace
(C0303)
[convention] 1018-1018: Trailing whitespace
(C0303)
[convention] 1023-1023: Trailing whitespace
(C0303)
[convention] 1028-1028: Trailing whitespace
(C0303)
[convention] 1046-1046: Trailing whitespace
(C0303)
[convention] 1054-1054: Trailing whitespace
(C0303)
[refactor] 963-963: Too many arguments (6/5)
(R0913)
[refactor] 963-963: Too many positional arguments (6/5)
(R0917)
[refactor] 963-963: Too many local variables (21/15)
(R0914)
surfsense_backend/app/tasks/connectors_indexing_tasks.py (1)
918-1157: Consider refactoring to reduce complexity and improve maintainability.The function has high complexity with 47 local variables, 6 nested blocks, and 117 statements. This makes it difficult to maintain and test.
Consider extracting helper methods to reduce complexity:
- Extract message processing logic:
async def _process_channel_messages(self, channel_id: str, channel_name: str, guild_name: str, start_date_str: str, end_date_str: str) -> Optional[Document]: # Handle message fetching, formatting, and document creation pass
- Extract guild processing logic:
async def _process_guild_channels(self, guild: dict, discord_client: DiscordConnector, start_date_str: str, end_date_str: str) -> Tuple[int, List[str]]: # Handle all channels in a guild passThis would improve readability, testability, and maintainability of the codebase.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 969-969: Line too long (122/100)
(C0301)
[convention] 1026-1026: Line too long (113/100)
(C0301)
[convention] 1039-1039: Line too long (106/100)
(C0301)
[convention] 1086-1086: Line too long (158/100)
(C0301)
[convention] 1092-1092: Line too long (104/100)
(C0301)
[convention] 1098-1098: Line too long (110/100)
(C0301)
[convention] 1125-1125: Line too long (136/100)
(C0301)
[convention] 1143-1143: Line too long (143/100)
(C0301)
[convention] 1147-1147: Line too long (113/100)
(C0301)
[refactor] 918-918: Too many local variables (47/15)
(R0914)
[warning] 1154-1154: Catching too general exception Exception
(W0718)
[warning] 955-955: Use lazy % formatting in logging functions
(W1203)
[warning] 966-966: Use lazy % formatting in logging functions
(W1203)
[warning] 969-969: Use lazy % formatting in logging functions
(W1203)
[warning] 987-987: Catching too general exception Exception
(W0718)
[warning] 981-981: Access to a protected member _bot_task of a client class
(W0212)
[warning] 982-982: Access to a protected member _wait_until_ready of a client class
(W0212)
[warning] 986-986: Use lazy % formatting in logging functions
(W1203)
[warning] 988-988: Use lazy % formatting in logging functions
(W1203)
[warning] 1000-1000: Use lazy % formatting in logging functions
(W1203)
[warning] 1127-1127: Catching too general exception Exception
(W0718)
[warning] 1004-1004: Use lazy % formatting in logging functions
(W1203)
[warning] 1019-1019: Catching too general exception Exception
(W0718)
[warning] 1020-1020: Use lazy % formatting in logging functions
(W1203)
[warning] 1026-1026: Use lazy % formatting in logging functions
(W1203)
[refactor] 936-1157: Too many nested blocks (6/5)
(R1702)
[warning] 1039-1039: Use lazy % formatting in logging functions
(W1203)
[warning] 1086-1086: Use lazy % formatting in logging functions
(W1203)
[warning] 1125-1125: Use lazy % formatting in logging functions
(W1203)
[warning] 1128-1128: Use lazy % formatting in logging functions
(W1203)
[warning] 1135-1135: Use lazy % formatting in logging functions
(W1203)
[warning] 1147-1147: Use lazy % formatting in logging functions
(W1203)
[warning] 1152-1152: Use lazy % formatting in logging functions
(W1203)
[warning] 1156-1156: Use lazy % formatting in logging functions
(W1203)
[refactor] 918-918: Too many return statements (7/6)
(R0911)
[refactor] 918-918: Too many branches (23/12)
(R0912)
[refactor] 918-918: Too many statements (117/50)
(R0915)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
surfsense_backend/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
surfsense_backend/app/tasks/connectors_indexing_tasks.py(2 hunks)surfsense_backend/app/utils/connector_service.py(1 hunks)surfsense_backend/pyproject.toml(1 hunks)surfsense_web/app/dashboard/[search_space_id]/connectors/add/discord-connector/page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- surfsense_backend/pyproject.toml
- surfsense_web/app/dashboard/[search_space_id]/connectors/add/discord-connector/page.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
surfsense_backend/app/tasks/connectors_indexing_tasks.py (3)
surfsense_backend/app/connectors/discord_connector.py (7)
DiscordConnector(18-310)start_bot(56-86)_wait_until_ready(111-128)get_guilds(130-156)close_bot(88-97)get_text_channels(158-188)get_channel_history(190-261)surfsense_backend/app/db.py (5)
SearchSourceConnector(143-153)SearchSourceConnectorType(55-63)Document(96-109)Chunk(111-118)DocumentType(44-53)surfsense_backend/app/utils/document_converters.py (1)
generate_content_hash(144-146)
🪛 Pylint (3.3.7)
surfsense_backend/app/tasks/connectors_indexing_tasks.py
[error] 15-15: Unable to import 'slack_sdk.errors'
(E0401)
[convention] 16-16: standard import "logging" should be placed before third party imports "sqlalchemy.ext.asyncio.AsyncSession", "sqlalchemy.exc.SQLAlchemyError", "sqlalchemy.future.select" (...) "app.connectors.linear_connector.LinearConnector", "app.connectors.discord_connector.DiscordConnector", "slack_sdk.errors.SlackApiError"
(C0411)
[convention] 17-17: standard import "asyncio" should be placed before third party imports "sqlalchemy.ext.asyncio.AsyncSession", "sqlalchemy.exc.SQLAlchemyError", "sqlalchemy.future.select" (...) "app.connectors.linear_connector.LinearConnector", "app.connectors.discord_connector.DiscordConnector", "slack_sdk.errors.SlackApiError"
(C0411)
[convention] 969-969: Line too long (122/100)
(C0301)
[convention] 1026-1026: Line too long (113/100)
(C0301)
[convention] 1039-1039: Line too long (106/100)
(C0301)
[convention] 1086-1086: Line too long (158/100)
(C0301)
[convention] 1092-1092: Line too long (104/100)
(C0301)
[convention] 1098-1098: Line too long (110/100)
(C0301)
[convention] 1125-1125: Line too long (136/100)
(C0301)
[convention] 1143-1143: Line too long (143/100)
(C0301)
[convention] 1147-1147: Line too long (113/100)
(C0301)
[refactor] 918-918: Too many local variables (47/15)
(R0914)
[warning] 1154-1154: Catching too general exception Exception
(W0718)
[warning] 955-955: Use lazy % formatting in logging functions
(W1203)
[warning] 966-966: Use lazy % formatting in logging functions
(W1203)
[warning] 969-969: Use lazy % formatting in logging functions
(W1203)
[warning] 987-987: Catching too general exception Exception
(W0718)
[warning] 981-981: Access to a protected member _bot_task of a client class
(W0212)
[warning] 982-982: Access to a protected member _wait_until_ready of a client class
(W0212)
[warning] 986-986: Use lazy % formatting in logging functions
(W1203)
[warning] 988-988: Use lazy % formatting in logging functions
(W1203)
[warning] 1000-1000: Use lazy % formatting in logging functions
(W1203)
[warning] 1127-1127: Catching too general exception Exception
(W0718)
[warning] 1004-1004: Use lazy % formatting in logging functions
(W1203)
[warning] 1019-1019: Catching too general exception Exception
(W0718)
[warning] 1020-1020: Use lazy % formatting in logging functions
(W1203)
[warning] 1026-1026: Use lazy % formatting in logging functions
(W1203)
[refactor] 936-1157: Too many nested blocks (6/5)
(R1702)
[warning] 1039-1039: Use lazy % formatting in logging functions
(W1203)
[warning] 1086-1086: Use lazy % formatting in logging functions
(W1203)
[warning] 1125-1125: Use lazy % formatting in logging functions
(W1203)
[warning] 1128-1128: Use lazy % formatting in logging functions
(W1203)
[warning] 1135-1135: Use lazy % formatting in logging functions
(W1203)
[warning] 1147-1147: Use lazy % formatting in logging functions
(W1203)
[warning] 1152-1152: Use lazy % formatting in logging functions
(W1203)
[warning] 1156-1156: Use lazy % formatting in logging functions
(W1203)
[refactor] 918-918: Too many return statements (7/6)
(R0911)
[refactor] 918-918: Too many branches (23/12)
(R0912)
[refactor] 918-918: Too many statements (117/50)
(R0915)
surfsense_backend/app/utils/connector_service.py
[convention] 962-962: Trailing whitespace
(C0303)
[convention] 963-963: Line too long (159/100)
(C0301)
[convention] 994-994: Trailing whitespace
(C0303)
[convention] 1018-1018: Trailing whitespace
(C0303)
[convention] 1023-1023: Trailing whitespace
(C0303)
[convention] 1028-1028: Trailing whitespace
(C0303)
[convention] 1046-1046: Trailing whitespace
(C0303)
[convention] 1054-1054: Trailing whitespace
(C0303)
[convention] 1057-1057: Trailing newlines
(C0305)
[refactor] 963-963: Too many arguments (6/5)
(R0913)
[refactor] 963-963: Too many positional arguments (6/5)
(R0917)
[refactor] 963-963: Too many local variables (21/15)
(R0914)
🔇 Additional comments (1)
surfsense_backend/app/tasks/connectors_indexing_tasks.py (1)
1090-1094: Maintain consistency with other connectors for summary generation.The Discord indexing function correctly uses the LLM for summary generation, which is consistent with other connectors like Slack, Notion, and Linear.
This approach ensures high-quality summaries across all connector types.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1092-1092: Line too long (104/100)
(C0301)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
surfsense_backend/app/tasks/connectors_indexing_tasks.py (1)
982-983:⚠️ Potential issueAvoid accessing protected members (duplicate issue)
This is a duplicate of a previous review comment. The code still directly accesses protected members
_bot_taskand_wait_until_ready, which violates encapsulation principles.As suggested in the previous review, consider refactoring the
DiscordConnectorclass to expose proper public methods for bot lifecycle management instead of accessing internal state directly.🧰 Tools
🪛 Pylint (3.3.7)
[warning] 982-982: Access to a protected member _bot_task of a client class
(W0212)
[warning] 983-983: Access to a protected member _wait_until_ready of a client class
(W0212)
surfsense_backend/app/connectors/discord_connector.py (2)
10-13: Fix import ordering (unresolved from previous review)This is still an unresolved issue from a previous review. Standard library imports should be placed before third-party imports according to PEP 8.
import logging -import discord -from discord.ext import commands import datetime import asyncio + +import discord +from discord.ext import commands🧰 Tools
🪛 Pylint (3.3.7)
[error] 10-10: Unable to import 'discord'
(E0401)
[error] 11-11: Unable to import 'discord.ext'
(E0401)
[convention] 12-12: standard import "datetime" should be placed before third party imports "discord", "discord.ext.commands"
(C0411)
[convention] 13-13: standard import "asyncio" should be placed before third party imports "discord", "discord.ext.commands"
(C0411)
115-118: 🛠️ Refactor suggestionReplace sleep workaround with proper synchronization (unresolved issue)
This is an unresolved issue from a previous review. The
asyncio.sleep(1)workaround is unreliable and can lead to race conditions.- # Give the event loop a chance to switch to the bot's startup task. - # This allows self.start() to begin initializing the client. - # Terrible solution, but necessary to avoid blocking the event loop. - await asyncio.sleep(1) # Yield control to the event loopThe
wait_until_ready()method should handle synchronization properly without needing the sleep workaround.
🧹 Nitpick comments (4)
surfsense_backend/app/tasks/connectors_indexing_tasks.py (2)
18-18: Remove unused importThe
ThreadPoolExecutorimport is not used anywhere in the code and should be removed to clean up dependencies.-from concurrent.futures import ThreadPoolExecutor🧰 Tools
🪛 Ruff (0.11.9)
18-18:
concurrent.futures.ThreadPoolExecutorimported but unusedRemove unused import:
concurrent.futures.ThreadPoolExecutor(F401)
🪛 Pylint (3.3.7)
[convention] 18-18: standard import "concurrent.futures.ThreadPoolExecutor" should be placed before third party imports "sqlalchemy.ext.asyncio.AsyncSession", "sqlalchemy.exc.SQLAlchemyError", "sqlalchemy.future.select" (...) "app.connectors.linear_connector.LinearConnector", "app.connectors.discord_connector.DiscordConnector", "slack_sdk.errors.SlackApiError"
(C0411)
[warning] 18-18: Unused ThreadPoolExecutor imported from concurrent.futures
(W0611)
919-1171: Consider refactoring complex function to improve maintainabilityThe function has significant complexity with 50 local variables, 6 levels of nesting, and over 120 statements. This makes it difficult to maintain and test effectively.
Consider extracting smaller helper functions for:
- Guild processing logic (lines 998-1145)
- Channel message processing (lines 1010-1140)
- Document creation and storage (lines 1091-1139)
This would improve readability, testability, and maintainability while reducing the cognitive load of the main function.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 970-970: Line too long (122/100)
(C0301)
[convention] 1027-1027: Line too long (113/100)
(C0301)
[convention] 1040-1040: Line too long (106/100)
(C0301)
[convention] 1087-1087: Line too long (158/100)
(C0301)
[convention] 1093-1093: Line too long (104/100)
(C0301)
[convention] 1139-1139: Line too long (136/100)
(C0301)
[convention] 1157-1157: Line too long (143/100)
(C0301)
[convention] 1161-1161: Line too long (113/100)
(C0301)
[refactor] 919-919: Too many local variables (50/15)
(R0914)
[warning] 1168-1168: Catching too general exception Exception
(W0718)
[warning] 956-956: Use lazy % formatting in logging functions
(W1203)
[warning] 967-967: Use lazy % formatting in logging functions
(W1203)
[warning] 970-970: Use lazy % formatting in logging functions
(W1203)
[warning] 988-988: Catching too general exception Exception
(W0718)
[warning] 982-982: Access to a protected member _bot_task of a client class
(W0212)
[warning] 983-983: Access to a protected member _wait_until_ready of a client class
(W0212)
[warning] 987-987: Use lazy % formatting in logging functions
(W1203)
[warning] 989-989: Use lazy % formatting in logging functions
(W1203)
[warning] 1001-1001: Use lazy % formatting in logging functions
(W1203)
[warning] 1141-1141: Catching too general exception Exception
(W0718)
[warning] 1005-1005: Use lazy % formatting in logging functions
(W1203)
[warning] 1020-1020: Catching too general exception Exception
(W0718)
[warning] 1021-1021: Use lazy % formatting in logging functions
(W1203)
[warning] 1027-1027: Use lazy % formatting in logging functions
(W1203)
[refactor] 937-1171: Too many nested blocks (6/5)
(R1702)
[warning] 1040-1040: Use lazy % formatting in logging functions
(W1203)
[warning] 1087-1087: Use lazy % formatting in logging functions
(W1203)
[warning] 1139-1139: Use lazy % formatting in logging functions
(W1203)
[warning] 1142-1142: Use lazy % formatting in logging functions
(W1203)
[warning] 1149-1149: Use lazy % formatting in logging functions
(W1203)
[warning] 1161-1161: Use lazy % formatting in logging functions
(W1203)
[warning] 1166-1166: Use lazy % formatting in logging functions
(W1203)
[warning] 1170-1170: Use lazy % formatting in logging functions
(W1203)
[refactor] 919-919: Too many return statements (7/6)
(R0911)
[refactor] 919-919: Too many branches (23/12)
(R0912)
[refactor] 919-919: Too many statements (120/50)
(R0915)
surfsense_backend/app/connectors/discord_connector.py (2)
124-124: Remove unnecessary f-string prefixThe f-string on line 124 doesn't contain any placeholders and should use a regular string.
- logger.error(f"Bot did not become ready within 60 seconds. Connection may have failed.") + logger.error("Bot did not become ready within 60 seconds. Connection may have failed.")🧰 Tools
🪛 Ruff (0.11.9)
124-124: f-string without any placeholders
Remove extraneous
fprefix(F541)
🪛 Pylint (3.3.7)
[warning] 124-124: Use lazy % formatting in logging functions
(W1203)
[warning] 124-124: Using an f-string that does not have any interpolated variables
(W1309)
41-41: Use lazy formatting in logging functionsMultiple logging statements use f-strings or % formatting which should be replaced with lazy formatting for better performance.
Example fixes:
- logger.info(f"Logged in as {self.user} (ID: {self.user.id})") + logger.info("Logged in as %s (ID: %s)", self.user, self.user.id) - logger.error(f"Privileged Intents Required: {e}. Make sure all required intents are enabled in your bot's application page.") + logger.error("Privileged Intents Required: %s. Make sure all required intents are enabled in your bot's application page.", e)Also applies to: 76-76, 80-80, 84-84
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 41-41: Use lazy % formatting in logging functions
(W1203)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
surfsense_backend/app/connectors/discord_connector.py(1 hunks)surfsense_backend/app/tasks/connectors_indexing_tasks.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
surfsense_backend/app/tasks/connectors_indexing_tasks.py (3)
surfsense_backend/app/connectors/discord_connector.py (7)
DiscordConnector(18-310)start_bot(57-86)_wait_until_ready(111-128)get_guilds(130-156)close_bot(88-97)get_text_channels(158-188)get_channel_history(190-261)surfsense_backend/app/db.py (5)
SearchSourceConnector(143-153)SearchSourceConnectorType(55-63)Document(96-109)Chunk(111-118)DocumentType(44-53)surfsense_backend/app/utils/document_converters.py (1)
generate_content_hash(144-146)
🪛 Ruff (0.11.9)
surfsense_backend/app/tasks/connectors_indexing_tasks.py
18-18: concurrent.futures.ThreadPoolExecutor imported but unused
Remove unused import: concurrent.futures.ThreadPoolExecutor
(F401)
surfsense_backend/app/connectors/discord_connector.py
124-124: f-string without any placeholders
Remove extraneous f prefix
(F541)
🪛 Pylint (3.3.7)
surfsense_backend/app/tasks/connectors_indexing_tasks.py
[error] 15-15: Unable to import 'slack_sdk.errors'
(E0401)
[convention] 16-16: standard import "logging" should be placed before third party imports "sqlalchemy.ext.asyncio.AsyncSession", "sqlalchemy.exc.SQLAlchemyError", "sqlalchemy.future.select" (...) "app.connectors.linear_connector.LinearConnector", "app.connectors.discord_connector.DiscordConnector", "slack_sdk.errors.SlackApiError"
(C0411)
[convention] 17-17: standard import "asyncio" should be placed before third party imports "sqlalchemy.ext.asyncio.AsyncSession", "sqlalchemy.exc.SQLAlchemyError", "sqlalchemy.future.select" (...) "app.connectors.linear_connector.LinearConnector", "app.connectors.discord_connector.DiscordConnector", "slack_sdk.errors.SlackApiError"
(C0411)
[convention] 18-18: standard import "concurrent.futures.ThreadPoolExecutor" should be placed before third party imports "sqlalchemy.ext.asyncio.AsyncSession", "sqlalchemy.exc.SQLAlchemyError", "sqlalchemy.future.select" (...) "app.connectors.linear_connector.LinearConnector", "app.connectors.discord_connector.DiscordConnector", "slack_sdk.errors.SlackApiError"
(C0411)
[warning] 18-18: Unused ThreadPoolExecutor imported from concurrent.futures
(W0611)
[convention] 970-970: Line too long (122/100)
(C0301)
[convention] 1027-1027: Line too long (113/100)
(C0301)
[convention] 1040-1040: Line too long (106/100)
(C0301)
[convention] 1087-1087: Line too long (158/100)
(C0301)
[convention] 1093-1093: Line too long (104/100)
(C0301)
[convention] 1139-1139: Line too long (136/100)
(C0301)
[convention] 1157-1157: Line too long (143/100)
(C0301)
[convention] 1161-1161: Line too long (113/100)
(C0301)
[refactor] 919-919: Too many local variables (50/15)
(R0914)
[warning] 1168-1168: Catching too general exception Exception
(W0718)
[warning] 956-956: Use lazy % formatting in logging functions
(W1203)
[warning] 967-967: Use lazy % formatting in logging functions
(W1203)
[warning] 970-970: Use lazy % formatting in logging functions
(W1203)
[warning] 988-988: Catching too general exception Exception
(W0718)
[warning] 982-982: Access to a protected member _bot_task of a client class
(W0212)
[warning] 983-983: Access to a protected member _wait_until_ready of a client class
(W0212)
[warning] 987-987: Use lazy % formatting in logging functions
(W1203)
[warning] 989-989: Use lazy % formatting in logging functions
(W1203)
[warning] 1001-1001: Use lazy % formatting in logging functions
(W1203)
[warning] 1141-1141: Catching too general exception Exception
(W0718)
[warning] 1005-1005: Use lazy % formatting in logging functions
(W1203)
[warning] 1020-1020: Catching too general exception Exception
(W0718)
[warning] 1021-1021: Use lazy % formatting in logging functions
(W1203)
[warning] 1027-1027: Use lazy % formatting in logging functions
(W1203)
[refactor] 937-1171: Too many nested blocks (6/5)
(R1702)
[warning] 1040-1040: Use lazy % formatting in logging functions
(W1203)
[warning] 1087-1087: Use lazy % formatting in logging functions
(W1203)
[warning] 1139-1139: Use lazy % formatting in logging functions
(W1203)
[warning] 1142-1142: Use lazy % formatting in logging functions
(W1203)
[warning] 1149-1149: Use lazy % formatting in logging functions
(W1203)
[warning] 1161-1161: Use lazy % formatting in logging functions
(W1203)
[warning] 1166-1166: Use lazy % formatting in logging functions
(W1203)
[warning] 1170-1170: Use lazy % formatting in logging functions
(W1203)
[refactor] 919-919: Too many return statements (7/6)
(R0911)
[refactor] 919-919: Too many branches (23/12)
(R0912)
[refactor] 919-919: Too many statements (120/50)
(R0915)
surfsense_backend/app/connectors/discord_connector.py
[convention] 33-33: Line too long (117/100)
(C0301)
[convention] 66-66: Line too long (107/100)
(C0301)
[convention] 72-72: Line too long (102/100)
(C0301)
[convention] 76-76: Line too long (137/100)
(C0301)
[convention] 110-110: Trailing whitespace
(C0303)
[convention] 119-119: Trailing whitespace
(C0303)
[convention] 186-186: Trailing whitespace
(C0303)
[convention] 230-230: Line too long (114/100)
(C0301)
[convention] 237-237: Line too long (115/100)
(C0301)
[convention] 254-254: Line too long (107/100)
(C0301)
[convention] 259-259: Trailing whitespace
(C0303)
[convention] 306-306: Line too long (136/100)
(C0301)
[error] 10-10: Unable to import 'discord'
(E0401)
[error] 11-11: Unable to import 'discord.ext'
(E0401)
[warning] 41-41: Use lazy % formatting in logging functions
(W1203)
[warning] 76-76: Use lazy % formatting in logging functions
(W1203)
[warning] 80-80: Use lazy % formatting in logging functions
(W1203)
[warning] 84-84: Use lazy % formatting in logging functions
(W1203)
[warning] 124-124: Use lazy % formatting in logging functions
(W1203)
[warning] 124-124: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 127-127: Use lazy % formatting in logging functions
(W1203)
[warning] 155-155: Use lazy % formatting in logging functions
(W1203)
[warning] 173-173: Use lazy % formatting in logging functions
(W1203)
[warning] 177-177: Use lazy % formatting in logging functions
(W1203)
[warning] 187-187: Use lazy % formatting in logging functions
(W1203)
[warning] 214-214: Use lazy % formatting in logging functions
(W1203)
[warning] 218-218: Use lazy % formatting in logging functions
(W1203)
[warning] 221-221: Use lazy % formatting in logging functions
(W1203)
[warning] 233-233: Use lazy % formatting in logging functions
(W1203)
[warning] 240-240: Use lazy % formatting in logging functions
(W1203)
[warning] 254-254: Use lazy % formatting in logging functions
(W1203)
[warning] 257-257: Use lazy % formatting in logging functions
(W1203)
[warning] 260-260: Use lazy % formatting in logging functions
(W1203)
[warning] 281-281: Use lazy % formatting in logging functions
(W1203)
[warning] 285-285: Use lazy % formatting in logging functions
(W1203)
[warning] 292-292: Use lazy % formatting in logging functions
(W1203)
[warning] 300-300: Use lazy % formatting in logging functions
(W1203)
[warning] 303-303: Use lazy % formatting in logging functions
(W1203)
[warning] 306-306: Use lazy % formatting in logging functions
(W1203)
[warning] 309-309: Use lazy % formatting in logging functions
(W1203)
[convention] 12-12: standard import "datetime" should be placed before third party imports "discord", "discord.ext.commands"
(C0411)
[convention] 13-13: standard import "asyncio" should be placed before third party imports "discord", "discord.ext.commands"
(C0411)
🔇 Additional comments (2)
surfsense_backend/app/tasks/connectors_indexing_tasks.py (1)
1095-1114: Good use of asyncio.to_thread for embedding operationsThe use of
asyncio.to_threadfor embedding operations is a good practice that prevents blocking the event loop during CPU-intensive embedding computations.surfsense_backend/app/connectors/discord_connector.py (1)
18-311: Overall implementation is well-structuredThe Discord connector implementation follows good practices with:
- Proper error handling for various Discord exceptions
- Comprehensive logging for debugging
- Good separation of concerns with distinct methods for different operations
- Appropriate use of Discord.py library features
The class provides a solid foundation for Discord integration despite the code quality issues that need addressing.
🧰 Tools
🪛 Ruff (0.11.9)
124-124: f-string without any placeholders
Remove extraneous
fprefix(F541)
🪛 Pylint (3.3.7)
[convention] 33-33: Line too long (117/100)
(C0301)
[convention] 66-66: Line too long (107/100)
(C0301)
[convention] 72-72: Line too long (102/100)
(C0301)
[convention] 76-76: Line too long (137/100)
(C0301)
[convention] 110-110: Trailing whitespace
(C0303)
[convention] 119-119: Trailing whitespace
(C0303)
[convention] 186-186: Trailing whitespace
(C0303)
[convention] 230-230: Line too long (114/100)
(C0301)
[convention] 237-237: Line too long (115/100)
(C0301)
[convention] 254-254: Line too long (107/100)
(C0301)
[convention] 259-259: Trailing whitespace
(C0303)
[convention] 306-306: Line too long (136/100)
(C0301)
[warning] 41-41: Use lazy % formatting in logging functions
(W1203)
[warning] 76-76: Use lazy % formatting in logging functions
(W1203)
[warning] 80-80: Use lazy % formatting in logging functions
(W1203)
[warning] 84-84: Use lazy % formatting in logging functions
(W1203)
[warning] 124-124: Use lazy % formatting in logging functions
(W1203)
[warning] 124-124: Using an f-string that does not have any interpolated variables
(W1309)
[warning] 127-127: Use lazy % formatting in logging functions
(W1203)
[warning] 155-155: Use lazy % formatting in logging functions
(W1203)
[warning] 173-173: Use lazy % formatting in logging functions
(W1203)
[warning] 177-177: Use lazy % formatting in logging functions
(W1203)
[warning] 187-187: Use lazy % formatting in logging functions
(W1203)
[warning] 214-214: Use lazy % formatting in logging functions
(W1203)
[warning] 218-218: Use lazy % formatting in logging functions
(W1203)
[warning] 221-221: Use lazy % formatting in logging functions
(W1203)
[warning] 233-233: Use lazy % formatting in logging functions
(W1203)
[warning] 240-240: Use lazy % formatting in logging functions
(W1203)
[warning] 254-254: Use lazy % formatting in logging functions
(W1203)
[warning] 257-257: Use lazy % formatting in logging functions
(W1203)
[warning] 260-260: Use lazy % formatting in logging functions
(W1203)
[warning] 281-281: Use lazy % formatting in logging functions
(W1203)
[warning] 285-285: Use lazy % formatting in logging functions
(W1203)
[warning] 292-292: Use lazy % formatting in logging functions
(W1203)
[warning] 300-300: Use lazy % formatting in logging functions
(W1203)
[warning] 303-303: Use lazy % formatting in logging functions
(W1203)
[warning] 306-306: Use lazy % formatting in logging functions
(W1203)
[warning] 309-309: Use lazy % formatting in logging functions
(W1203)
| self._bot_task = None # Holds the async bot task | ||
| self._is_running = False # Flag to track if the bot is running |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making protected members public for proper encapsulation
The _bot_task and _wait_until_ready attributes are accessed from external code (indexing tasks), which violates encapsulation. Consider exposing these through public methods instead.
For example:
+ def get_bot_task(self):
+ """Get the current bot task."""
+ return self._bot_task
+
+ def set_bot_task(self, task):
+ """Set the bot task."""
+ self._bot_task = task
+
+ async def wait_until_ready_public(self):
+ """Public method to wait until bot is ready."""
+ return await self._wait_until_ready()📝 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._bot_task = None # Holds the async bot task | |
| self._is_running = False # Flag to track if the bot is running | |
| self._bot_task = None # Holds the async bot task | |
| self._is_running = False # Flag to track if the bot is running | |
| def get_bot_task(self): | |
| """Get the current bot task.""" | |
| return self._bot_task | |
| def set_bot_task(self, task): | |
| """Set the bot task.""" | |
| self._bot_task = task | |
| async def wait_until_ready_public(self): | |
| """Public method to wait until bot is ready.""" | |
| return await self._wait_until_ready() |
🤖 Prompt for AI Agents
In surfsense_backend/app/connectors/discord_connector.py around lines 35 to 36,
the attributes _bot_task and _wait_until_ready are currently protected but
accessed externally, breaking encapsulation. To fix this, rename these
attributes to be private (prefix with double underscore) and provide public
getter methods to access their values safely, ensuring proper encapsulation and
controlled access.
|
Looks good to me. This is a major contribution. Thanks @MuhamadAjiW AWESOME WORK 🥇 |
Feat/Discord Connector
Feat/Discord Connector
2 participants
Description
Motivation and Context
I am highly interested in this project. Self-hosted knowledge base is awesome. Aside from that, I need the functionality since I use discord quite intensely
Changes Overview
It's working so far, but may require plenty of tuning.
Screenshots
Query test
Edit view
Add view
Setup view 1
Setup view 2
Types of changes
Testing
Checklist:
Hopefully this is useful, thank you and nice meeting you all!
Summary by CodeRabbit
New Features
Improvements
Chores