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(autofix): Tie code changes to the step #1687

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jennmueng
Copy link
Member

@jennmueng jennmueng commented Jan 6, 2025

Instead of storing the file_changes in the root of the state, it stores it inside the changes step. Doing so now entirely contains the plan+code workflow inside the changes step. This includes moving the insights from the default step to the changes step. This now makes us entirely consolidate things related to a workflow to a single step. Will follow up with moving root cause into a single step as well, and entirely remove the notion of having a "default" step for insights and another step for the result.

Method of migration

So we'll be migrating all existing Autofix runs to this new model, we have a migration that migrates all run states over by copying over the file changes and codebase state to this new more organized format. However, we won't be deleting the existing fields to allow for current frontends to still render fine and also allow for ease of rolling back in case something goes wrong.

@jennmueng jennmueng force-pushed the jenn/autofix/changes-tied-to-step branch from b19fba2 to ad4ab89 Compare January 15, 2025 16:00
@@ -204,6 +200,45 @@ def ask_user_question(self, question: str):

cur.status = AutofixStatus.WAITING_FOR_USER_RESPONSE

def add_user_message(self, message: str, memory_index: int):
Copy link
Member Author

Choose a reason for hiding this comment

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

tacked this change on for cleanliness

@@ -38,18 +38,6 @@
RepoIdentifiers = tuple[RepoExternalId, RepoInternalId]


class AutofixCodebaseStateManager(CodebaseStateManager):
Copy link
Member Author

Choose a reason for hiding this comment

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

unused

self.event_manager = event_manager
self.state = state

# TODO: Remove this when we no longer need the backwards compatibility.
self.event_manager.migrate_step_keys()
Copy link
Member Author

Choose a reason for hiding this comment

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

no longer need

Comment on lines -75 to -81
with state.update() as cur:
for repo in request.repos:
if repo.external_id not in cur.codebases:
cur.codebases[repo.external_id] = CodebaseState(
file_changes=[],
repo_external_id=repo.external_id,
)
Copy link
Member Author

Choose a reason for hiding this comment

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

this is no longer needed as the new codebase states are initialized inside event_manager.send_coding_start

@jennmueng jennmueng force-pushed the jenn/autofix/changes-tied-to-step branch from 4b72e9a to a8ca349 Compare January 27, 2025 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant