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: Use AI parsing to obtain more accurate filenames and contents #94

Closed
wants to merge 1 commit into from

Conversation

NoCLin
Copy link
Contributor

@NoCLin NoCLin commented Jun 17, 2023

No description provided.

@patillacode
Copy link
Collaborator

Hi @NoCLin

thanks for this!
Code looks good, although I haven't been able to give it a go.

Could you maybe give us some examples of it being used somehow?

I think this is interesting, we could even maybe have a collection of "classic" functions, in its own file... just an idea.

Thoughts on this @AntonOsika ?

@NoCLin
Copy link
Contributor Author

NoCLin commented Jun 17, 2023

@patillacode Thanks.

here is the parsed result:

(venv) ➜  gpt-engineer git:(main) ✗ gpt-engineer example --model "gpt-3.5-turbo-0613"            

Sure! Let's start by defining the core classes and their purposes:

  1. Model: Represents the state and logic of the game.
  2. View: Responsible for rendering the game on the screen.
  3. Controller: Handles user input and updates the model accordingly.

Now, let's create the necessary files and implement the code for each component.

  1. model.py
from dataclasses import dataclass
from typing import List, Tuple

@dataclass
class Snake:
    body: List[Tuple[int, int]]
    direction: Tuple[int, int]

@dataclass
class Food:
    position: Tuple[int, int]

@dataclass
class Game:
    width: int
    height: int
    snake: Snake
    food: Food
    score: int

    def update(self):
        # Update the game state based on the current direction of the snake
        pass

    def check_collision(self):
        # Check for collisions with walls, snake body, and food
        pass

    def move_snake(self):
        # Move the snake in the current direction
        pass

    def eat_food(self):
        # Increase the score and generate new food
        pass
  1. view.py
import pygame

class View:
    def __init__(self, game):
        self.game = game
        pygame.init()
        self.screen = pygame.display.set_mode((game.width, game.height))
        pygame.display.set_caption("Snake Game")

    def render(self):
        # Render the game state on the screen
        pass

    def draw_snake(self):
        # Draw the snake on the screen
        pass

    def draw_food(self):
        # Draw the food on the screen
        pass

    def draw_score(self):
        # Draw the score on the screen
        pass

    def update_screen(self):
        # Update the screen display
        pass
  1. controller.py
import pygame
from pygame.locals import *

class Controller:
    def __init__(self, game):
        self.game = game

    def handle_events(self):
        for event in pygame.event.get():
            if event.type == QUIT:
                pygame.quit()
                sys.exit()
            elif event.type == KEYDOWN:
                if event.key == K_UP:
                    self.game.snake.direction = (0, -1)
                elif event.key == K_DOWN:
                    self.game.snake.direction = (0, 1)
                elif event.key == K_LEFT:
                    self.game.snake.direction = (-1, 0)
                elif event.key == K_RIGHT:
                    self.game.snake.direction = (1, 0)
  1. main.py (entry point)
from model import Game
from view import View
from controller import Controller

def main():
    game = Game(width=800, height=600, snake=None, food=None, score=0)
    view = View(game)
    controller = Controller(game)

    while True:
        controller.handle_events()
        game.update()
        view.render()
        view.update_screen()

if __name__ == "__main__":
    main()
This is a basic implementation of the Snake game using the MVC architecture. The model represents the game state and logic, the view handles rendering, and the controller handles user input. The main.py file serves as the entry point for the game.try to parse chat by ai
saving  model.py
saving  view.py
saving  controller.py
saving  main.py

As the upstream updated, something was broken, especially in the execute_workspace step.
I will keep this PR up-to-date.

@NoCLin
Copy link
Contributor Author

NoCLin commented Jun 17, 2023

If we use AI for the newly added execute_workspace stage, we will need to use a different prompt/function.

If you guys think this parsing method is okay, I will update the PR along with the latest upstream commit.

@patillacode
Copy link
Collaborator

My two cents:

I like the idea, I was thinking of adding something similar to this myself, my function looked like this:

functions: [
        {
            name: "write_files",
            description: "Output files and files content",
            parameters: {
                type: "object",
                properties: {
                    filenames: {
                        type: "array",
                        items: { type: "string" },
                        description: "List of filenames"
                    },
                    contents: {
                        type: "array",
                        items: { type: "string" },
                        description: "List of contents for each file"
                    }
                },
                required: ["filename", "content"]
            },
            function_call: "auto"
        }
    ]

Very similar 😅

I would recommend something like this, but since it is a drastic change on the handling of the GTP output, therefore I wanted to check with you.

This would get some issues off the table like #35 (and all the similar ones)
(cc @goncalomoita)

@AntonOsika @FOLLGAD (should I ping someone else?)
If you think this is a good idea I'll do a in-depth review of this PR.

Thoughts?

@NoCLin
Copy link
Contributor Author

NoCLin commented Jun 18, 2023

Hi @patillacode , we could deep dive on this.

I like your function name write_files and I think

{  
  files: [ 
     { "filename":"a","content":"1"},
     { "filename":"a","content":"2"},
   ] 
}

would be more semantic than

{
   "filenames":["a","b"],
   "contents":["1","2"]
}

I'm not a core contributor yet, but I aspire to be., I don't know your future plans. There may be some structural adjustments needed.

@patillacode
Copy link
Collaborator

I have taken some inspiration from jpenalbae/olgpt (thanks @jpenalbae

Notes to consider:

  • It might be better to force the call to the function through the promt instead of using auto

  • Sometimes gpt returns the code as text instead of properly formatted as we would expect with the function.

  • When you have more than one file you gotta call the function as many times as files you want to generate, meaning you gotta give the model the proper context, which is dirty and more expensive (more tokens).
    So we could call the function just once with the list of all files and their content.

Please have a look @NoCLin

@AntonOsika
Copy link
Collaborator

Great to see the thought put in this @NoCLin

Before we merge something and I take time to review in depth I always ask:

What exact problem is this PR addressing?

@NoCLin
Copy link
Contributor Author

NoCLin commented Jun 18, 2023

@patillacode

It might be better to force the call to the function through the promt instead of using auto

I can't agree more.

Sometimes gpt returns the code as text instead of properly formatted as we would expect with the function.

I believe this approach of PR will be more accurate compared to requesting GPT to output and parse based on regular expressions, as GPT may not return in the format we desire.

When you have more than one file you gotta call the function as many times as files you want to generate, meaning you gotta give the model the proper context, which is dirty and more expensive (more tokens).
So we could call the function just once with the list of all files and their content.

I apologize for any confusion, but are you referring to the need for the function I provided to be called multiple times?

{  
  files: [ 
     { "filename":"a","content":"1"},
     { "filename":"a","content":"2"},
   ] 
}

⬆️
The aforementioned parameter structure could result in the parsing of multiple files within a single request.

BTW, I would like update my code before merged.

@AntonOsika As OpenAI released the function calling ability, we can leverage this feature to achieve parsing that is more accurate than regular expressions.

@patillacode
Copy link
Collaborator

Yes, please @NoCLin

Make the updates you want, when you are ready for a review please assign me as a reviewer.
I think we are pretty aligned on this one.

Make sure to pull the latest changes! A recent PR modified a lot in this part of the code.

@AntonOsika this makes the issues with file creations obsolete, the solution is more reliable than #120 AFAIU

@NoCLin
Copy link
Contributor Author

NoCLin commented Jun 18, 2023

Yes, please @NoCLin

Make the updates you want, when you are ready for a review please assign me as a reviewer. I think we are pretty aligned on this one.

Make sure to pull the latest changes! A recent PR modified a lot in this part of the code.

@AntonOsika this makes the issues with file creations obsolete, the solution is more reliable than #120 AFAIU

Thanks, I need rebase first.

@NoCLin NoCLin force-pushed the main branch 2 times, most recently from f95a83b to 7d08219 Compare June 18, 2023 14:08
@NoCLin
Copy link
Contributor Author

NoCLin commented Jun 18, 2023

@patillacode Please kindly take a look as I have no permission to assign reviewers.

btw, the following code would block the process. One failure leads to all failures is not a good option.

https://github.com/AntonOsika/gpt-engineer/blob/ef6a752f5e539c65557c47311bcb3c7162b87edd/gpt_engineer/steps.py#L173-L178

OpenAI has been invoked in multiple instances and I believe it can be changed after finishing #154.

@patillacode patillacode self-requested a review June 18, 2023 14:17
@patillacode
Copy link
Collaborator

Will do at some point, please be patient 😄

btw, you should be able to request for a review anyway.

@NoCLin
Copy link
Contributor Author

NoCLin commented Jun 18, 2023

Will do at some point, please be patient 😄

btw, you should be able to request for a review anyway.

Thanks! Really? I dont know how to request a review if I have no write access...

Anyone who can assist me with verification?

@patillacode
Copy link
Collaborator

@AntonOsika things like #169 would also be fixed when we get this done.

@AntonOsika
Copy link
Collaborator

Thanks @NoCLin for the good work put in here and other PRs!

I think this is already solved since yesterday though?

Could we wait until we see more issues on this since yesterday, and if see we take a look at this?

Philosophy is still:

As few lines of code as possible.

@NoCLin
Copy link
Contributor Author

NoCLin commented Jun 19, 2023

Thanks @NoCLin for the good work put in here and other PRs!

I think this is already solved since yesterday though?

Could we wait until we see more issues on this since yesterday, and if see we take a look at this?

Philosophy is still:

As few lines of code as possible.

I believe this feature is worth merging.

btw, due to the rapid upstream changes, I have already performed multiple rebases. I would like rebase it one more time.

@NoCLin
Copy link
Contributor Author

NoCLin commented Jun 19, 2023

@patillacode @AntonOsika could you please have a look?

To mitigate potential upstream changes, I have retained some TODOs. I will make the necessary modifications as soon as the merge is completed.

@patillacode
Copy link
Collaborator

I really think this is the way to go @AntonOsika

@AntonOsika
Copy link
Collaborator

Thanks again, rebasing is tricky. It's also a way to make the workflow faster.

We only merge code that:

  • Addresses a clear known issue
  • Don't have TODO comments.

Will close this Junlin, but let's use the ideas/copy paste code from it in future PRs when we have clear problems to address <3

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.

None yet

4 participants