-
Notifications
You must be signed in to change notification settings - Fork 244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes for 1.0.18 #570
Changes for 1.0.18 #570
Conversation
ctx = SESSION_CONTEXT.get() | ||
cwd = ctx.cwd | ||
llm_api_handler = ctx.llm_api_handler | ||
|
||
# Use print because stream is not initialized yet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using the logging framework instead of print
for consistency across the codebase and more control over logging levels and destinations.
ctx = SESSION_CONTEXT.get() | ||
cwd = ctx.cwd | ||
llm_api_handler = ctx.llm_api_handler | ||
|
||
# Use print because stream is not initialized yet | ||
print("Scanning codebase for updates...") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The warning message about not being a git repository could potentially be confusing for users if their project is not intended to be a git repository. Consider adding a check to only display this warning if git-related features are being used.
intervals = parse_intervals(interval) | ||
for _interval in intervals: | ||
feature = CodeFeature(cwd / path, _interval) | ||
if not interval: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's great to see handling for the case where there's no interval specified, simplifying the creation of a CodeFeature
for a whole file. This makes the function more robust and easier to understand.
@@ -42,7 +42,7 @@ class Config: | |||
) | |||
provider: Optional[str] = attr.field(default=None, metadata={"auto_completions": ["openai", "anthropic", "azure"]}) | |||
embedding_model: str = attr.field( | |||
default="text-embedding-ada-002", | |||
default="text-embedding-3-large", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating the default embedding_model
to text-embedding-3-large
aligns with the latest models available, potentially offering better performance or features.
@@ -217,7 +217,7 @@ async def call_llm_api( | |||
return response | |||
|
|||
@api_guard | |||
def call_embedding_api(self, input_texts: list[str], model: str = "text-embedding-ada-002") -> EmbeddingResponse: | |||
def call_embedding_api(self, input_texts: list[str], model: str = "text-embedding-3-large") -> EmbeddingResponse: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating the default model for embedding API calls to text-embedding-3-large
is consistent with the change in config.py
. This ensures that the embedding calls use the updated model by default.
@@ -164,7 +164,6 @@ async def _main(self): | |||
|
|||
await session_context.llm_api_handler.initialize_client() | |||
|
|||
print("Scanning codebase for updates...") | |||
await code_context.refresh_daemon() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the print statement here is a good move for cleanliness, especially since the update scanning message has been moved to CodeContext
. This helps in keeping the session initialization cleaner.
MENTAT CODE REVIEW IN ACTIVE DEVELOPMENT. Only in use on mentat and internal repos. This pull request introduces several key updates and improvements. The addition of a warning when not in a git repository and the update to the default embedding model are notable changes. There are suggestions for further refinement, such as using the logging framework for consistency and ensuring user clarity with the git repository warning. Overall, these changes are positive and align with maintaining up-to-date defaults and improving user feedback. |
Most of #569
Pull Request Checklist