-
Notifications
You must be signed in to change notification settings - Fork 52
CG-10805 & CG-10806: Migrate Everything to Langgraph + Multi-LLM Config (openai + anthropic) #600
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
Conversation
codegen-bot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files |
The error handling in the LLM class could be improved by adding more specific error types and messages for common failure scenarios (rate limits, token limits, etc). Consider adding a custom exception hierarchy. |
Args: | ||
model_provider: "anthropic" or "openai" | ||
model_name: Name of the model to use | ||
**kwargs: Additional configuration options. Supported options: |
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 adding validation for model names against a list of supported models for each provider. This would help catch configuration errors early.
- Search within specific directories | ||
- Filter by file extensions | ||
- Get paginated results | ||
memory = MemorySaver() if memory else None |
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 memory implementation could benefit from configurable cleanup/expiry policies to prevent memory leaks in long-running sessions.
from langchain_core.language_models.chat_models import BaseChatModel | ||
from langchain_core.messages import BaseMessage | ||
from langchain_core.outputs import ChatResult | ||
from langchain_core.runnables import Runnable |
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 docstrings for the new LLM class methods could be more detailed, especially regarding the expected behavior when switching between different model providers.
@@ -0,0 +1,155 @@ | |||
"""Tests for the unified LLM class.""" |
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.
While the LLM class has good test coverage, consider adding integration tests that verify the interaction between the LLM, agent, and memory components.
Overall Assessment: This PR represents a significant improvement to the codebase's LLM integration architecture. The changes make the system more flexible and maintainable by:
The code quality is good, with clear separation of concerns and proper error handling. The migration to langgraph's ReAct agent is a positive architectural decision that should improve reliability and maintainability. Suggested improvements have been noted in inline comments, focusing on:
These suggestions are relatively minor and could be addressed in follow-up PRs. The current changes are well-structured and ready for merging after addressing any critical inline comments. |
🎉 This PR is included in version 0.32.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
yoooooo lets get it bro 🔥🔥🔥🔥🔥🔥🔥 |
migrated everything over to langgarph
now uses LLM class for everything including the model in
semantic_edit.py
no more AgentExecutor
The syntax for codeagent stays the same
the agent streams each step
agent.stream
now instead of calling invoke so that intermediate steps can be printedIf you want to directly invoke the agent, then use thread_id instead + set debug to true for the
create_codebase_agent
function so that logs get printed. However, streaming is preferred and outputs nicer logs as well.agent.stream
does NOT mean streaming each token! Streaming indicates getting each agent action one by one.agent.invoke
won't output until the the agent finishes the executionlanggraph uses threads to keep track of the conversation history
If you want to call invoke for whatever reason, then you can set debug flag to true for printed debug statements.
However, streaming shows nicer outputs. Streaming also does NOT mean streaming each token. agent.stream means to stream each step, and thus we can print each step however we want.