Add MCP Config Settings - #106
Conversation
|
Review these changes at https://app.gitnotebooks.com/AlphaSphereDotAI/chattr/pull/106 |
Reviewer's GuideRefactors settings to support JSON-based MCP configuration via a file path with validation, simplifies default URL fields, consolidates MCP settings, and adds error handling for MCP client failures in graph building. Class diagram for updated MCPSettings and Settings classesclassDiagram
class MCPSettings {
FilePath path
+is_json() Self
+check_mcp_config() Self
}
class Settings {
ModelSettings model
MemorySettings memory
VectorDatabaseSettings vector_database
MCPSettings mcp
DirectorySettings directory
}
Settings --> MCPSettings : mcp
Settings --> ModelSettings : model
Settings --> MemorySettings : memory
Settings --> VectorDatabaseSettings : vector_database
Settings --> DirectorySettings : directory
Class diagram for updated MemorySettings and VectorDatabaseSettings defaultsclassDiagram
class MemorySettings {
RedisDsn url = "redis://localhost:6379"
}
class VectorDatabaseSettings {
StrictStr name = "chattr"
HttpUrl url = "http://localhost:6333"
}
Flow diagram for MCP config file validation and loadingflowchart TD
A[User provides MCP config file path] --> B{Is file extension .json?}
B -- No --> C[Raise ValueError: must be JSON]
B -- Yes --> D[Read file contents]
D --> E[Parse JSON into config dict]
E --> F[Print config]
F --> G[Return MCPSettings instance]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Warning Rate limit exceeded@MH0386 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 29 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughException handling was added to the Changes
Sequence Diagram(s)sequenceDiagram
participant Graph as GraphBuilder
participant Memory as Memory Abstraction
participant Model as LLM Model
participant Logger as Logger
Graph->>Memory: Initialize Memory instance (_setup_memory)
Graph->>Graph: Setup tools (_setup_tools)
alt Tool setup success
Graph-->>Graph: Tools list
else Tool setup failure
Graph->>Logger: Log warning
Graph-->>Graph: Return empty list
end
User->>Graph: Generate response with State input
Graph->>Memory: Search user memories by user ID and latest message
Memory-->>Graph: Return relevant memories
Graph->>Model: Call model with system prompt + memory context
Model-->>Graph: Model response
Graph->>Memory: Add model response to memory with user ID
Graph-->>User: Return response
sequenceDiagram
participant User as User
participant MCPSettings as MCPSettings Validator
participant FileSystem as JSON File
User->>MCPSettings: Provide MCP config file path
MCPSettings->>FileSystem: Check file extension (.json)
alt Valid extension
MCPSettings->>FileSystem: Load and parse JSON
MCPSettings->>MCPSettings: Validate JSON against schema
alt Valid JSON
MCPSettings-->>User: Accept config
else Invalid JSON
MCPSettings-->>User: Raise validation error
end
else Invalid extension
MCPSettings-->>User: Raise validation error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://cdn.github.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @MH0386, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request centralizes the configuration for the Multi-Server Control Plane (MCP) by introducing a single, file-based configuration approach. This change enhances the flexibility and maintainability of MCP settings. Additionally, I've improved the system's robustness by adding error handling for MCP client communication, ensuring that the application can gracefully handle scenarios where the MCP service is unavailable.
Highlights
- Refactored MCP Configuration: The MCPSettings class has been refactored to accept a single file path for a JSON configuration, moving away from individual fields like name, url, and transport.
- Added Configuration Validation: Validation logic has been added to MCPSettings to ensure the provided path points to a valid JSON file.
- Consolidated MCP Settings: The main Settings class now uses a single 'mcp' field, consolidating previously separate voice and video generator MCP settings.
- Improved Error Handling for MCP Client: Error handling has been implemented in the _setup_tools function to gracefully manage cases where the MCP client is unavailable, logging a warning and returning an empty list.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
|
Here's the code health analysis summary for commits Analysis Summary
|
![sourcery-ai[bot]](https://cdn.github.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @MH0386 - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/chattr/settings.py:75` </location>
<code_context>
- transport: Literal["sse", "stdio", "streamable_http", "websocket"] = Field(
- default=None
- )
+ path: FilePath = Field(default=None)
+
+ @model_validator(mode="after")
</code_context>
<issue_to_address>
Using FilePath with a default of None may cause type issues.
Consider changing the type to Optional[FilePath] if None is intended as a valid value, to avoid type or validation errors.
</issue_to_address>
### Comment 2
<location> `src/chattr/settings.py:93` </location>
<code_context>
+ | StreamableHttpConnection
+ | WebsocketConnection,
+ ] = loads(self.path.read_text(encoding="utf-8"))
+ print(config)
+ print()
+ return self
</code_context>
<issue_to_address>
Debug print statements should be removed from production code.
Print statements in configuration validation may clutter logs or expose sensitive data. Please remove them or use appropriate logging if necessary.
</issue_to_address>
### Comment 3
<location> `src/chattr/graph/builder.py:182` </location>
<code_context>
list[BaseTool]: A list of BaseTool objects retrieved from the MCP client.
"""
- return await _mcp_client.get_tools()
+ try:
+ return await _mcp_client.get_tools()
+ except Exception as e:
+ logger.warning(f"MCP unavailable: {e}")
+ return []
def draw_graph(self) -> None:
</code_context>
<issue_to_address>
Catching all exceptions may mask underlying issues.
Catching only specific exceptions, such as network-related errors, will help prevent masking unrelated bugs.
</issue_to_address>
### Comment 4
<location> `src/chattr/settings.py:74` </location>
<code_context>
+ url: HttpUrl = Field(default="http://localhost:6333")
class MCPSettings(BaseModel):
- name: StrictStr = Field(default=None)
- url: HttpUrl = Field(default=None)
</code_context>
<issue_to_address>
Consider moving file I/O and JSON parsing out of the MCPSettings model into a separate loader function to keep the model focused on data validation.
Here’s one way to drastically simplify `MCPSettings` by pulling all file-I/O and JSON parsing out into a tiny loader function, and keeping the Pydantic model strictly focused on its final shape:
```python
# mcp_loader.py
from pathlib import Path
from json import loads
def load_mcp_config(path: Path) -> dict:
if path.suffix.lower() != ".json":
raise ValueError("MCP config file must be a .json")
text = path.read_text(encoding="utf-8")
return loads(text)
```
```python
# settings.py
from pathlib import Path
from typing import Literal, List
from pydantic import BaseModel, Field
from .mcp_loader import load_mcp_config
class MCPSettings(BaseModel):
name: str
url: HttpUrl
transport: Literal["sse", "stdio", "streamable_http", "websocket"]
args: List[str] = []
class Settings(BaseSettings):
# pick up the path from an ENV or default location
mcp_path: Path = Field(default=Path("mcp_config.json"))
mcp: MCPSettings = Field(
default_factory=lambda: MCPSettings(**load_mcp_config(Settings.__config__.env_file))
)
```
What this gives you:
- **No in-model I/O** or `@model_validator` complexity.
- A single, tiny loader with just one responsibility.
- A lean `MCPSettings` that only declares the final fields and types.
- All existing behavior (validate “.json”, parse JSON) stays intact.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| print(config) | ||
| print() |
There was a problem hiding this comment.
issue: Debug print statements should be removed from production code.
Print statements in configuration validation may clutter logs or expose sensitive data. Please remove them or use appropriate logging if necessary.
| try: | ||
| return await _mcp_client.get_tools() | ||
| except Exception as e: | ||
| logger.warning(f"MCP unavailable: {e}") | ||
| return [] |
There was a problem hiding this comment.
suggestion (bug_risk): Catching all exceptions may mask underlying issues.
Catching only specific exceptions, such as network-related errors, will help prevent masking unrelated bugs.
There was a problem hiding this comment.
Pull Request Overview
This PR simplifies MCP (Model Context Protocol) configuration by replacing individual service configurations with a unified JSON file-based approach. The changes consolidate multiple MCP service settings into a single configuration file path and add error handling for MCP unavailability.
- Replaced individual MCP service configurations with a single JSON file path configuration
- Added validation to ensure MCP config files are JSON format
- Added exception handling in MCP client setup to gracefully handle unavailable services
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/chattr/settings.py | Refactored MCPSettings to use a single JSON config file instead of individual service configurations |
| src/chattr/graph/builder.py | Added exception handling for MCP client tool retrieval |
| print(config) | ||
| print() |
There was a problem hiding this comment.
Debug print statements should be removed from production code. Consider using the logger instead or removing these statements entirely.
| print(config) | |
| print() | |
| logger.debug(config) |
| print(config) | ||
| print() |
There was a problem hiding this comment.
Debug print statements should be removed from production code. Consider using the logger instead or removing these statements entirely.
| print(config) | |
| print() | |
| logger.info(config) |
|
|
||
| class MemorySettings(BaseModel): | ||
| url: RedisDsn = Field(default=RedisDsn(url="redis://localhost:6379")) | ||
| url: RedisDsn = Field(default="redis://localhost:6379") |
There was a problem hiding this comment.
The default value should be wrapped in RedisDsn() constructor to maintain type consistency with the field annotation.
| url: RedisDsn = Field(default="redis://localhost:6379") | |
| url: RedisDsn = Field(default=RedisDsn("redis://localhost:6379")) |
| class VectorDatabaseSettings(BaseModel): | ||
| name: StrictStr = Field(default="chattr") | ||
| url: HttpUrl = Field(default=HttpUrl(url="http://localhost:6333")) | ||
| url: HttpUrl = Field(default="http://localhost:6333") |
There was a problem hiding this comment.
The default value should be wrapped in HttpUrl() constructor to maintain type consistency with the field annotation.
| url: HttpUrl = Field(default="http://localhost:6333") | |
| url: HttpUrl = Field(default=HttpUrl("http://localhost:6333", scheme="http", host="localhost", tld="")) |
There was a problem hiding this comment.
Code Review
This pull request refactors the MCP configuration to be loaded from a single JSON file, which is a good simplification. It also adds error handling for when MCP tools are unavailable. My main concern is that the code in src/chattr/graph/builder.py has not been updated to use this new configuration method, which will cause the application to break as it still references the old, now-removed settings. Additionally, there are some debugging artifacts (print statements) in src/chattr/settings.py, and the loaded MCP configuration is not actually stored or used. Please see my detailed comments for suggestions.
| def check_mcp_config(self) -> Self: | ||
| if self.path: | ||
| config: dict[ | ||
| str, | ||
| StdioConnection | ||
| | SSEConnection | ||
| | StreamableHttpConnection | ||
| | WebsocketConnection, | ||
| ] = loads(self.path.read_text(encoding="utf-8")) | ||
| print(config) | ||
| print() | ||
| return self |
There was a problem hiding this comment.
This validator loads the configuration from the file, but the config variable is local and its value is discarded when the method returns. For this feature to be useful, the loaded configuration needs to be stored on the MCPSettings instance so it can be accessed by other parts of the application. You could consider adding a pydantic.PrivateAttr to the MCPSettings model and assigning the loaded config to it within this validator.
| print(config) | ||
| print() |
There was a problem hiding this comment.
These print statements appear to be for debugging and should be removed from the code before merging.
| try: | ||
| return await _mcp_client.get_tools() | ||
| except Exception as e: | ||
| logger.warning(f"MCP unavailable: {e}") |
There was a problem hiding this comment.
To aid in debugging, it's helpful to log the full traceback when an exception occurs. You can do this by adding exc_info=True. Also, consider catching a more specific exception type on the line above instead of the generic Exception if possible, to avoid masking other potential issues.
| logger.warning(f"MCP unavailable: {e}") | |
| logger.warning(f"MCP unavailable: {e}", exc_info=True) |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/chattr/settings.py (2)
75-75: Fix type annotation for optional FilePath field.Using FilePath with a default of None may cause type issues. Consider changing the type to Optional[FilePath] if None is intended as a valid value, to avoid type or validation errors.
- path: FilePath = Field(default=None) + path: FilePath | None = Field(default=None)
83-95: Address multiple issues in check_mcp_config validator.Several issues need attention:
- Debug print statements should be removed from production code
- Missing error handling for file I/O and JSON parsing
- Complex file I/O logic belongs in a separate function
@model_validator(mode="after") def check_mcp_config(self) -> Self: if self.path: - config: dict[ - str, - StdioConnection - | SSEConnection - | StreamableHttpConnection - | WebsocketConnection, - ] = loads(self.path.read_text(encoding="utf-8")) - print(config) - print() + try: + config: dict[str, StdioConnection | SSEConnection | StreamableHttpConnection | WebsocketConnection] = loads( + self.path.read_text(encoding="utf-8") + ) + logger.debug(f"Loaded MCP config: {config}") + except (OSError, FileNotFoundError) as e: + raise ValueError(f"Failed to read MCP config file: {e}") + except Exception as e: + raise ValueError(f"Failed to parse MCP config JSON: {e}") return selfsrc/chattr/graph/builder.py (1)
182-186: Consider catching specific exceptions for better error handling.While the exception handling improves fault tolerance, catching all exceptions may mask underlying issues that should be addressed differently.
Consider catching specific exceptions that are expected during MCP client initialization:
try: return await _mcp_client.get_tools() - except Exception as e: - logger.warning(f"MCP unavailable: {e}") + except (ConnectionError, TimeoutError, OSError) as e: + logger.warning(f"Failed to retrieve MCP tools - service unavailable: {e}") + return [] + except Exception as e: + logger.error(f"Unexpected error retrieving MCP tools: {e}") return []This approach:
- Handles expected network/connectivity issues gracefully with a warning
- Logs unexpected errors at error level for investigation
- Prevents masking of programming errors
🧹 Nitpick comments (1)
src/chattr/settings.py (1)
77-81: Improve file extension validation.The current validation only checks for exact ".json" but should be case-insensitive to handle ".JSON" files as well.
- def is_json(self) -> Self: - if self.path and self.path.suffix != ".json": - raise ValueError("MCP config file must be a JSON file") - return self + def is_json(self) -> Self: + if self.path and self.path.suffix.lower() != ".json": + raise ValueError("MCP config file must be a JSON file") + return self
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/chattr/graph/builder.py(1 hunks)src/chattr/settings.py(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🔇 Additional comments (1)
src/chattr/settings.py (1)
3-3: LGTM! Import changes support the new MCP configuration approach.The new imports are appropriately added to support JSON parsing, self-referencing type hints, and MCP connection type definitions.
Also applies to: 6-6, 10-15, 20-20
🔍 Vulnerabilities of
|
| digest | sha256:6fa7a52c76ef88cd51d2a77937e5d4de2d9f16866e4c7f245ad8915856e07359 |
| vulnerabilities |     |
| platform | linux/amd64 |
| size | 274 MB |
| packages | 360 |
📦 Base Image python:1e02be40c22aa1c20a4ae404c529966193ebfc54beb8ff6a863062c853aa94f3
| also known as |
|
| digest | sha256:27e5dbf4794217dd490e049cf356cc95654d28b88ce0bb675af2d8320ef4640a |
| vulnerabilities |   |
# Dockerfile (41:41)
COPY --from=builder --chown=app:app --chmod=555 /app/.venv /app/.venv
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
Description
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
Description
Description
Description
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Dockerfile (32:37)
RUN groupadd app && \
useradd -m -g app -s /bin/bash app && \
apt-get update > /dev/null && \
apt-get install -y --no-install-recommends curl > /dev/null && \
apt-get clean > /dev/null && \
rm -rf /var/lib/apt/lists/*
Description
|
Recommended fixes for image
|
| Name | 3.13.5-slim-bookworm |
| Digest | sha256:27e5dbf4794217dd490e049cf356cc95654d28b88ce0bb675af2d8320ef4640a |
| Vulnerabilities | |
| Pushed | 1 month ago |
| Size | 44 MB |
| Packages | 139 |
| Flavor | debian |
| OS | 12 |
| Runtime | 3.13.5 |
| Slim | ✅ |
The base image is also available under the supported tag(s):3-slim-bookworm,3.13-slim,3.13-slim-bookworm,3.13.5-slim,3.13.5-slim-bookworm,slim,slim-bookworm
Refresh base image
Rebuild the image using a newer base image version. Updating this may result in breaking changes.| Tag | Details | Pushed | Vulnerabilities |
|---|---|---|---|
3-slimNewer image for same tag Also known as:
|
Benefits:
|
1 month ago |  |
Change base image
| Tag | Details | Pushed | Vulnerabilities |
|---|---|---|---|
alpineTag is preferred tag Also known as:
|
Benefits:
|
1 month ago |   |
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/chattr/graph/builder.py (1)
139-142: Fix missing voice_generator_mcp in Settings or refactor to use generic MCPSettingsThe
GraphBuilder(src/chattr/graph/builder.py:139–142) still callscls.settings.voice_generator_mcp.name: SSEConnection( url=str(cls.settings.voice_generator_mcp.url), transport=cls.settings.voice_generator_mcp.transport, ),but
voice_generator_mcpis no longer defined onSettings(we only havemcp: MCPSettings). This will cause an attribute‐error at runtime.Please update one of the following:
• Re-introduce a dedicated field in
Settingsforvoice_generator_mcpif you still need it.
• Or consolidate fully by refactoring these lines to load the SSE endpoint from the genericsettings.mcp(e.g. parse yourmcp-config.jsonand pull out the “voice_generator” entry).Example diff (pseudo‐code):
- cls.settings.voice_generator_mcp.name: SSEConnection( - url=str(cls.settings.voice_generator_mcp.url), - transport=cls.settings.voice_generator_mcp.transport, - ), + # Load “voice_generator” endpoint from generic MCP config + endpoint = Settings.mcp.load_endpoint("voice_generator") + endpoint.name: SSEConnection( + url=str(endpoint.url), + transport=endpoint.transport, + ),
♻️ Duplicate comments (2)
src/chattr/graph/builder.py (2)
213-217: Consider catching specific exceptions.As noted in previous reviews, catching specific exceptions would be better than catching all exceptions, which may mask underlying issues.
51-59: Fix potential None value passed to constructor.The
toolsvariable is initialized toNoneand may remainNoneif an exception occurs. However, the constructor expectslist[BaseTool], notOptional[list[BaseTool]].Apply this fix to ensure tools is always a list:
- tools = None + tools = [] memory = await cls._setup_memory() try: tools: list[BaseTool] = await cls._setup_tools( MultiServerMCPClient(cls._create_mcp_config()) ) except Exception as e: - logger.warning(f"Failed to setup tools: {e}") + logger.warning(f"Failed to retrieve MCP tools: {e}") return cls(memory, tools)The error message improvement aligns with past review feedback about being more specific.
🧹 Nitpick comments (1)
src/chattr/graph/state.py (1)
4-7: LGTM! Consider enhancing the docstring.The State class correctly extends MessagesState with the mem0_user_id field. Consider expanding the docstring to explain what mem0_user_id represents and its role in the memory management system.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
assets/mcp-config.json(1 hunks)mcp.json(1 hunks)pyproject.toml(1 hunks)src/chattr/graph/builder.py(8 hunks)src/chattr/graph/state.py(1 hunks)src/chattr/settings.py(3 hunks)src/chattr/utils.py(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/chattr/utils.py
- assets/mcp-config.json
- pyproject.toml
- mcp.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/chattr/settings.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/chattr/graph/builder.py (2)
src/chattr/graph/state.py (1)
State(4-7)src/chattr/settings.py (1)
Settings(129-143)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and push Docker image
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/docker.yaml (1)
115-118: Pin the Qdrant image to a specific version for reproducible CI
qdrant/qdrant:latestcan introduce breaking changes at any push; last week’s v1.8.4 introduced an incompatible gRPC port default. Pinning to a concrete tag (e.g.,qdrant/qdrant:1.8.5) keeps the workflow deterministic and protects the PR against unrelated CI breakage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
.github/workflows/docker.yaml(1 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and push Docker image
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@Mergifyio queue |
🟠 Waiting for conditions to matchDetails
|
|
|
@Mergifyio queue |
✅ The pull request has been merged automaticallyDetailsThe pull request has been merged automatically at c0aa1eb |
|
Thank you for your contribution @MH0386! Your pull request has been merged. |
Summary by Sourcery
Introduce file-based configuration for MCP settings and unify MCP endpoint definitions, simplify default URL assignments, and add error handling for MCP tool loading in the graph builder.
New Features:
Enhancements: