Skip to content
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

feat(backend): Migrate old models in existing agents #9452

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

kcze
Copy link
Contributor

@kcze kcze commented Feb 10, 2025

Some existing nodes use models that no longer exist as values on LlmModel enum.

Changes 🏗️

  • Update non existing models to gpt-4o when loading AgentNode from_db
  • Set default models for AI blocks to gpt-4o

Checklist 📋

For code changes:

  • I have clearly listed my changes in the PR description
  • I have made a test plan
  • I have tested my changes according to the test plan:
    • ...

@kcze kcze requested a review from majdyz February 10, 2025 12:30
@kcze kcze requested a review from a team as a code owner February 10, 2025 12:30
@kcze kcze requested review from Bentlybro and removed request for a team February 10, 2025 12:30
Copy link

netlify bot commented Feb 10, 2025

Deploy Preview for auto-gpt-docs-dev canceled.

Name Link
🔨 Latest commit a37f223
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs-dev/deploys/67a9f181ce00bd0008e23a1c

Copy link

deepsource-io bot commented Feb 10, 2025

Here's the code health analysis summary for commits 610be98..a37f223. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource JavaScript LogoJavaScript✅ SuccessView Check ↗
DeepSource Python LogoPython✅ SuccessView Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

Copy link

netlify bot commented Feb 10, 2025

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit a37f223
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/67a9f1817adb5c0008c53fcc

@@ -71,6 +72,9 @@ class NodeModel(Node):
def from_db(node: AgentNode):
if not node.AgentBlock:
raise ValueError(f"Invalid node {node.id}, invalid AgentBlock.")

NodeModel.migrate_llm_models(node)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we do that here autogpt_platform/backend/backend/server/rest_api.py

@contextlib.asynccontextmanager
async def lifespan_context(app: fastapi.FastAPI):
    await backend.data.db.connect()
    await backend.data.block.initialize_blocks()
    await backend.data.user.migrate_and_encrypt_user_integrations()
    await backend.data.graph.fix_llm_provider_credentials()
    with launch_darkly_context():
        yield
    await backend.data.db.disconnect()

Copy link
Contributor

@Swiftyos Swiftyos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general happy with this approach. However, I don't feel we should be migrating ai models with clearly communicating it to the user.

Can we add a ticket to push a notification to the user when a model has to be migrated and we can address this at an appropriate time

@majdyz majdyz removed their request for review February 13, 2025 10:26
@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Feb 13, 2025
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflicts Automatically applied to PRs with merge conflicts platform/backend AutoGPT Platform - Back end platform/blocks size/m
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants