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

Add the ability to capture and load snapshots of memory and message history #192

Closed
wants to merge 1 commit into from

Conversation

Joe0
Copy link

@Joe0 Joe0 commented Apr 4, 2023

Related to:
#9 #122 #124 #125

Primary discussion here:
#124 (comment)

TODO:
Tests
Code review
Improve code quality

@@ -114,10 +114,10 @@ def chat_with_ai(
)

# Update full message history
full_message_history.append(
message_history.append(
Copy link

Choose a reason for hiding this comment

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

Is this a significant or required change?

@@ -178,14 +177,14 @@ def get_hyperlinks(url):

def commit_memory(string):
_text = f"""Committing memory with string "{string}" """
mem.permanent_memory.append(string)
memory.append(string)
Copy link

Choose a reason for hiding this comment

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

Nice, the stuff in this area makes sense to me.

class DataStore():

def persist_message_history(self, id):
return True
Copy link

Choose a reason for hiding this comment

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

What about return False on everything for the abstract base class? Or if errors are involved, throwing the NotImplementedError?

message_history.set_history(f["message_history"])
return True
except Exception as e:
return e
Copy link

Choose a reason for hiding this comment

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

This function returns either True or Error. Both a consistent boolean or an actual exception is better if you ask me.

@@ -248,6 +252,9 @@ def parse_arguments():
parser.add_argument('--speak', action='store_true', help='Enable Speak Mode')
parser.add_argument('--debug', action='store_true', help='Enable Debug Mode')
parser.add_argument('--gpt3only', action='store_true', help='Enable GPT3.5 Only Mode')
parser.add_argument('--enable-snapshots', action='store_true', help='Enable Snapshots')
parser.add_argument('--snapshot-path', type=str, default=None, required=False, help='Path for the snapshot directory')
parser.add_argument('--snapshot-id', type=str, default=None, required=False, help='ID of the snapshot to load')
Copy link

Choose a reason for hiding this comment

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

This all looks good right here

@nponeccop
Copy link
Contributor

Can you rebase it on the current master?

@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Apr 17, 2023
@github-actions
Copy link
Contributor

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

@Joe0 Joe0 closed this Apr 17, 2023
Say383 pushed a commit to Say383/Auto-GPT that referenced this pull request Sep 8, 2023
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants