-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
Conversation
Hi @NoCLin thanks for this! 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 Thanks. here is the parsed result:
Sure! Let's start by defining the core classes and their purposes:
Now, let's create the necessary files and implement the code for each component.
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
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
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)
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()
As the upstream updated, something was broken, especially in the |
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. |
My two cents: I like the idea, I was thinking of adding something similar to this myself, my function looked like this:
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) @AntonOsika @FOLLGAD (should I ping someone else?) Thoughts? |
Hi @patillacode , we could deep dive on this. I like your function name
would be more semantic than
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. |
I have taken some inspiration from jpenalbae/olgpt (thanks @jpenalbae Notes to consider:
Please have a look @NoCLin |
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? |
I can't agree more.
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.
I apologize for any confusion, but are you referring to the need for the function I provided to be called multiple times?
⬆️ 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. |
Yes, please @NoCLin Make the updates you want, when you are ready for a review please assign me as a reviewer. 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. |
f95a83b
to
7d08219
Compare
@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. gpt-engineer/gpt_engineer/steps.py Lines 173 to 178 in ef6a752
OpenAI has been invoked in multiple instances and I believe it can be changed after finishing #154. |
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? |
@AntonOsika things like #169 would also be fixed when we get this done. |
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. |
@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. |
I really think this is the way to go @AntonOsika |
Thanks again, rebasing is tricky. It's also a way to make the workflow faster. We only merge code that:
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 |
No description provided.