gpt-engineer icon indicating copy to clipboard operation
gpt-engineer copied to clipboard

feat: Use AI parsing to obtain more accurate filenames and contents

Open NoCLin opened this issue 1 year ago • 4 comments

NoCLin avatar Jun 17 '23 08:06 NoCLin

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 ?

patillacode avatar Jun 17 '23 11:06 patillacode

@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 avatar Jun 17 '23 12:06 NoCLin

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.

NoCLin avatar Jun 17 '23 12:06 NoCLin

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?

patillacode avatar Jun 17 '23 14:06 patillacode

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.

NoCLin avatar Jun 18 '23 00:06 NoCLin

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

patillacode avatar Jun 18 '23 02:06 patillacode

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?

AntonOsika avatar Jun 18 '23 06:06 AntonOsika

@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.

NoCLin avatar Jun 18 '23 10:06 NoCLin

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

patillacode avatar Jun 18 '23 12:06 patillacode

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 avatar Jun 18 '23 12:06 NoCLin

@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.

NoCLin avatar Jun 18 '23 14:06 NoCLin

Will do at some point, please be patient 😄

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

patillacode avatar Jun 18 '23 14:06 patillacode

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?

NoCLin avatar Jun 18 '23 14:06 NoCLin

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

patillacode avatar Jun 18 '23 22:06 patillacode

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.

AntonOsika avatar Jun 19 '23 10:06 AntonOsika

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 avatar Jun 19 '23 13:06 NoCLin

@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.

NoCLin avatar Jun 19 '23 14:06 NoCLin

I really think this is the way to go @AntonOsika

patillacode avatar Jun 19 '23 15:06 patillacode

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

AntonOsika avatar Jun 20 '23 11:06 AntonOsika