From 656337d5151bf9b0d2e40b39471dcd93e2f52e5d Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Mon, 9 Oct 2023 14:51:55 +1100 Subject: [PATCH 01/21] `parse_task.prompt` includes list of running processes, `kill_process` added to functions --- pilot/const/function_calls.py | 4 ++++ pilot/helpers/agents/Developer.py | 6 ++++-- pilot/helpers/cli.py | 19 ++++++++-------- pilot/prompts/development/parse_task.prompt | 9 +++++++- pilot/prompts/test_prompts.py | 24 +++++++++++++++++++++ 5 files changed, 49 insertions(+), 13 deletions(-) diff --git a/pilot/const/function_calls.py b/pilot/const/function_calls.py index 967cfaa95..5c764a68b 100644 --- a/pilot/const/function_calls.py +++ b/pilot/const/function_calls.py @@ -187,6 +187,10 @@ def command_definition(description_command=f'A single command that needs to be e 'description': 'Type of the development step that needs to be done to complete the entire task.', }, 'command': command_definition(), + 'kill_process': { + 'type': 'string', + 'description': 'To kill a process that was left running by a previous `command` provide the `process_name` in this field.' + }, 'code_change': { 'type': 'object', 'description': 'A code change that needs to be implemented. This should be used only if the task is of a type "code_change".', diff --git a/pilot/helpers/agents/Developer.py b/pilot/helpers/agents/Developer.py index b8dd47230..0cfcf13da 100644 --- a/pilot/helpers/agents/Developer.py +++ b/pilot/helpers/agents/Developer.py @@ -11,7 +11,7 @@ from helpers.Agent import Agent from helpers.AgentConvo import AgentConvo from utils.utils import should_execute_step, array_of_objects_to_string, generate_app_data -from helpers.cli import run_command_until_success, execute_command_and_check_cli_response +from helpers.cli import run_command_until_success, execute_command_and_check_cli_response, running_processes from const.function_calls import FILTER_OS_TECHNOLOGIES, EXECUTE_COMMANDS, GET_TEST_TYPE, IMPLEMENT_TASK from database.database import save_progress, get_progress_steps, update_app_status from utils.utils import get_os_info @@ -331,7 +331,9 @@ def continue_development(self, iteration_convo, last_branch_name, continue_descr # self.debugger.debug(iteration_convo, user_input=user_feedback) - task_steps = iteration_convo.send_message('development/parse_task.prompt', {}, IMPLEMENT_TASK) + task_steps = iteration_convo.send_message('development/parse_task.prompt', { + 'running_processes': running_processes + }, IMPLEMENT_TASK) iteration_convo.remove_last_x_messages(2) return self.execute_task(iteration_convo, task_steps, is_root_task=True) diff --git a/pilot/helpers/cli.py b/pilot/helpers/cli.py index c2f72f0cc..3ea38966d 100644 --- a/pilot/helpers/cli.py +++ b/pilot/helpers/cli.py @@ -17,8 +17,8 @@ interrupted = False -running_processes: Dict[str, int] = {} -"""Holds a list of process IDs, mapped to the `process_name` provided in the call to `execute_command()`.""" +running_processes: Dict[str, tuple[str, int]] = {} +"""Holds a list of (command, process ID)s, mapped to the `process_name` provided in the call to `execute_command()`.""" def enqueue_output(out, q): @@ -74,12 +74,12 @@ def run_command(command, root_path, q_stdout, q_stderr) -> subprocess.Popen: def terminate_named_process(process_name: str) -> None: if process_name in running_processes: - terminate_process(running_processes[process_name], process_name) + terminate_process(running_processes[process_name][1], process_name) def terminate_running_processes(): for process_name in list(running_processes.keys()): - terminate_process(running_processes[process_name], process_name) + terminate_process(running_processes[process_name][1], process_name) def terminate_process(pid: int, name=None) -> None: @@ -100,7 +100,7 @@ def terminate_process(pid: int, name=None) -> None: logger.error(f'Error while terminating process: {e}') for process_name in list(running_processes.keys()): - if running_processes[process_name] == pid: + if running_processes[process_name][1] == pid: del running_processes[process_name] @@ -174,7 +174,7 @@ def execute_command(project, command, timeout=None, process_name: str = None, fo if process_name is not None: terminate_named_process(process_name) - running_processes[process_name] = process.pid + running_processes[process_name] = (command, process.pid) output = '' stderr_output = '' @@ -233,7 +233,7 @@ def execute_command(project, command, timeout=None, process_name: str = None, fo logger.error('CLI ERROR: ' + stderr_line) if process_name is not None: - logger.info(f'Process {process_name} running as pid: {process.pid}') + logger.info(f'Process "{process_name}" running as pid: {process.pid}') break except (KeyboardInterrupt, TimeoutError) as e: @@ -248,8 +248,7 @@ def execute_command(project, command, timeout=None, process_name: str = None, fo terminate_process(process.pid) elapsed_time = time.time() - start_time - print(f'{command} took {round(elapsed_time * 1000)}ms to execute.') - logger.info(f'{command} took {round(elapsed_time * 1000)}ms to execute.') + logger.info(f'`{command}` took {round(elapsed_time * 1000)}ms to execute.') # stderr_output = '' # while not q_stderr.empty(): @@ -357,7 +356,7 @@ def run_command_until_success(convo, command, force=force) if response is None: - logger.info(f'{command} exit code: {exit_code}') + logger.info(f'`{command}` exit code: {exit_code}') if exit_code is None: response = 'DONE' else: diff --git a/pilot/prompts/development/parse_task.prompt b/pilot/prompts/development/parse_task.prompt index 9232259e5..799263d12 100644 --- a/pilot/prompts/development/parse_task.prompt +++ b/pilot/prompts/development/parse_task.prompt @@ -1 +1,8 @@ -Ok, now, take your previous message and convert it to actionable items. An item might be a code change or a command run. When you need to change code, make sure that you put the entire content of the file in the value of `content` key even though you will likely copy and paste the most of the previous message. \ No newline at end of file +Ok, now, take your previous message and convert it to actionable items. An item might be a code change or a command run. When you need to change code, make sure that you put the entire content of the file in the value of `content` key even though you will likely copy and paste the most of the previous message. +{%- if running_processes %} +Note that the following processes are already running: + +{% for key, data in running_processes.items() -%} +- "{{ key }}" (`{{ data[0] }}`) +{% endfor -%} +{% endif -%} diff --git a/pilot/prompts/test_prompts.py b/pilot/prompts/test_prompts.py index 51c7180d5..87e2d75f6 100644 --- a/pilot/prompts/test_prompts.py +++ b/pilot/prompts/test_prompts.py @@ -43,3 +43,27 @@ def test_prompt_ran_command_0_exit(): If the command was successfully executed, respond with `DONE`. If it wasn't, respond with `NEEDS_DEBUGGING`. '''.strip() + + +def test_parse_task_no_processes(): + # When + prompt = get_prompt('development/parse_task.prompt', { + 'running_processes': {} + }) + + # Then + assert 'the following processes' not in prompt + + +def test_parse_task_with_processes(): + # When + prompt = get_prompt('development/parse_task.prompt', { + 'running_processes': { + 'app': ('npm start', 123), + 'mongo': ('mongod', 456) + } + }) + + # Then + assert 'the following processes are already running:' in prompt + assert '- "app" (`npm start`)\n- "mongo" (`mongod`)' in prompt From c4400a471bc9b3667c43df70f0f68916d2cbf88f Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Mon, 9 Oct 2023 14:52:31 +1100 Subject: [PATCH 02/21] exponential back-off on retries after rate limiting --- pilot/utils/llm_connection.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/pilot/utils/llm_connection.py b/pilot/utils/llm_connection.py index f75064bb3..73ac86b6e 100644 --- a/pilot/utils/llm_connection.py +++ b/pilot/utils/llm_connection.py @@ -157,6 +157,8 @@ def set_function_error(args, err_str: str): del args[0]['function_buffer'] def wrapper(*args, **kwargs): + wait_duration_ms = None + while True: try: # spinner_stop(spinner) @@ -212,9 +214,13 @@ def wrapper(*args, **kwargs): match = re.search(r"Please try again in (\d+)ms.", err_str) if match: # spinner = spinner_start(colored("Rate limited. Waiting...", 'yellow')) - logger.debug('Rate limited. Waiting...') - wait_duration = int(match.group(1)) / 1000 - time.sleep(wait_duration) + if wait_duration_ms is None: + wait_duration_ms = int(match.group(1)) + elif wait_duration_ms < 6000: + # waiting 6ms isn't usually long enough - exponential back-off until about 6 seconds + wait_duration_ms *= 2 + logger.debug(f'Rate limited. Waiting {wait_duration_ms}ms...') + time.sleep(wait_duration_ms / 1000) continue print(red(f'There was a problem with request to openai API:')) From cb94bf1a9c0fba8816afa9a7a5cfed5ca0b45f88 Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Mon, 9 Oct 2023 14:53:13 +1100 Subject: [PATCH 03/21] include OS in `define_user_review_goal.prompt` --- pilot/helpers/agents/Developer.py | 5 ++++- pilot/prompts/development/define_user_review_goal.prompt | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pilot/helpers/agents/Developer.py b/pilot/helpers/agents/Developer.py index 0cfcf13da..6749ab6af 100644 --- a/pilot/helpers/agents/Developer.py +++ b/pilot/helpers/agents/Developer.py @@ -1,3 +1,4 @@ +import platform import uuid from utils.style import green, red, green_bold, yellow_bold, red_bold, blue_bold, white_bold from helpers.exceptions.TokenLimitError import TokenLimitError @@ -166,7 +167,9 @@ def task_postprocessing(self, convo, development_task, continue_development, tas if development_task is not None: convo.remove_last_x_messages(2) - detailed_user_review_goal = convo.send_message('development/define_user_review_goal.prompt', {}) + detailed_user_review_goal = convo.send_message('development/define_user_review_goal.prompt', { + 'os': platform.system() + }) convo.remove_last_x_messages(2) try: diff --git a/pilot/prompts/development/define_user_review_goal.prompt b/pilot/prompts/development/define_user_review_goal.prompt index 4dfc01310..2f9e94060 100644 --- a/pilot/prompts/development/define_user_review_goal.prompt +++ b/pilot/prompts/development/define_user_review_goal.prompt @@ -1,4 +1,4 @@ -How can a human user test if this task was completed successfully? If you specify a command that needs to be run or give example, be very specific. You don't want the user to have to think anything through but rather that they just follow your instructions. +How can a human user test if this task was completed successfully? If you specify a command that needs to be run or give example, be very specific. You don't want the user to have to think anything through but rather that they just follow your instructions. Note that the command will run on a {{ os }} machine. !IMPORTANT! In case the task can be tested by making an API request, do not suggest how can a request be made with Postman but rather write a full cURL command that the user can just run. From 85e302f655b5fa227bd5890cf07338db160b24df Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Mon, 9 Oct 2023 16:50:39 +1100 Subject: [PATCH 04/21] test_rate_limit_error --- pilot/utils/llm_connection.py | 5 +--- pilot/utils/test_llm_connection.py | 43 +++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/pilot/utils/llm_connection.py b/pilot/utils/llm_connection.py index 73ac86b6e..4e9fcf056 100644 --- a/pilot/utils/llm_connection.py +++ b/pilot/utils/llm_connection.py @@ -332,11 +332,8 @@ def return_result(result_data, lines_printed): stream=True ) - # Log the response status code and message - logger.debug(f'Response status code: {response.status_code}') - if response.status_code != 200: - logger.info(f'problem with request: {response.text}') + logger.info(f'problem with request (status {response.status_code}): {response.text}') raise Exception(f"API responded with status code: {response.status_code}. Response text: {response.text}") # function_calls = {'name': '', 'arguments': ''} diff --git a/pilot/utils/test_llm_connection.py b/pilot/utils/test_llm_connection.py index 0d9e79b87..cdbb276b8 100644 --- a/pilot/utils/test_llm_connection.py +++ b/pilot/utils/test_llm_connection.py @@ -2,7 +2,7 @@ from json import JSONDecodeError import pytest -from unittest.mock import patch, Mock +from unittest.mock import call, patch, Mock from dotenv import load_dotenv from jsonschema import ValidationError from const.function_calls import ARCHITECTURE, DEVELOPMENT_PLAN @@ -364,6 +364,47 @@ class TestLlmConnection: def setup_method(self): builtins.print, ipc_client_instance = get_custom_print({}) + @patch('utils.llm_connection.requests.post') + @patch('utils.llm_connection.time.sleep') + def test_rate_limit_error(self, mock_sleep, mock_post, monkeypatch): + monkeypatch.setenv('OPENAI_API_KEY', 'secret') + + error_text = '''{ + "error": { + "message": "Rate limit reached for 10KTPM-200RPM in organization org-OASFC7k1Ff5IzueeLArhQtnT on tokens per min. Limit: 10000 / min. Please try again in 6ms. Contact us through our help center at help.openai.com if you continue to have issues.", + "type": "tokens", + "param": null, + "code": "rate_limit_exceeded" + } + }''' + content = 'DONE' + success_text = '{"id": "gen-123", "choices": [{"index": 0, "delta": {"role": "assistant", "content": "' + content + '"}}]}' + + error_response = Mock() + error_response.status_code = 429 + error_response.text = error_text + + mock_response = Mock() + mock_response.status_code = 200 + mock_response.iter_lines.return_value = [success_text.encode('utf-8')] + + mock_post.side_effect = [error_response, error_response, error_response, error_response, error_response, + error_response, error_response, error_response, error_response, error_response, + error_response, error_response, mock_response] + wrapper = retry_on_exception(stream_gpt_completion) + data = {'model': 'gpt-4'} + + # When + response = wrapper(data, 'test', project) + + # Then + assert response == {'text': 'DONE'} + # assert mock_sleep.call_count == 9 + assert mock_sleep.call_args_list == [call(0.006), call(0.012), call(0.024), call(0.048), call(0.096), + call(0.192), call(0.384), call(0.768), call(1.536), call(3.072), + call(6.144), call(6.144)] + # mock_sleep.call + @patch('utils.llm_connection.requests.post') def test_stream_gpt_completion(self, mock_post, monkeypatch): # Given streaming JSON response From 9da702dc5bc0587a849837ba99d5cc023be23e5f Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Mon, 9 Oct 2023 19:21:59 +1100 Subject: [PATCH 05/21] When running the app check for `success_message`. If `process_name` is given, the app will remain running either way, but won't have to wait for timeout if we see the success message. --- pilot/const/function_calls.py | 8 ++++++-- pilot/helpers/agents/Developer.py | 2 ++ pilot/helpers/cli.py | 33 ++++++++++++++++++++----------- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/pilot/const/function_calls.py b/pilot/const/function_calls.py index 5c764a68b..7bfd6898a 100644 --- a/pilot/const/function_calls.py +++ b/pilot/const/function_calls.py @@ -44,7 +44,7 @@ def command_definition(description_command=f'A single command that needs to be e description_timeout= 'Timeout in milliseconds that represent the approximate time this command takes to finish. ' 'If you need to run a command that doesnt\'t finish by itself (eg. a command to run an app), ' - 'set the timeout to -1 and provide a process_name. ' + 'set the timeout to to a value long enough to determine that it has started successfully and provide a process_name. ' 'If you need to create a directory that doesn\'t exist and is not the root project directory, ' 'always create it by running a command `mkdir`'): return { @@ -59,6 +59,10 @@ def command_definition(description_command=f'A single command that needs to be e 'type': 'number', 'description': description_timeout, }, + 'success_message': { + 'type': 'string', + 'description': 'A message to look for in the output of the command to determine if successful or not.', + }, 'process_name': { 'type': 'string', 'description': 'If the process needs to continue running after the command is executed provide ' @@ -460,7 +464,7 @@ def command_definition(description_command=f'A single command that needs to be e }, 'path': { 'type': 'string', - 'description': 'Path of the file that needs to be saved on the disk.', + 'description': 'Full path of the file with the file name that needs to be saved.', }, 'content': { 'type': 'string', diff --git a/pilot/helpers/agents/Developer.py b/pilot/helpers/agents/Developer.py index 6749ab6af..eab246b81 100644 --- a/pilot/helpers/agents/Developer.py +++ b/pilot/helpers/agents/Developer.py @@ -98,10 +98,12 @@ def step_command_run(self, convo, step, i): additional_message = 'Let\'s start with the step #0:\n\n' if i == 0 else f'So far, steps { ", ".join(f"#{j}" for j in range(i)) } are finished so let\'s do step #{i + 1} now.\n\n' process_name = data['process_name'] if 'process_name' in data else None + success_message = data['success_message'] if 'success_message' in data else None return run_command_until_success(convo, data['command'], timeout=data['timeout'], process_name=process_name, + success_message=success_message, additional_message=additional_message) def step_human_intervention(self, convo, step: dict): diff --git a/pilot/helpers/cli.py b/pilot/helpers/cli.py index 3ea38966d..121df32ad 100644 --- a/pilot/helpers/cli.py +++ b/pilot/helpers/cli.py @@ -104,7 +104,8 @@ def terminate_process(pid: int, name=None) -> None: del running_processes[process_name] -def execute_command(project, command, timeout=None, process_name: str = None, force=False): +def execute_command(project, command, timeout=None, success_message=None, process_name: str = None, force=False) \ + -> (str, str, int): """ Execute a command and capture its output. @@ -112,6 +113,7 @@ def execute_command(project, command, timeout=None, process_name: str = None, fo project: The project associated with the command. command (str): The command to run. timeout (int, optional): The maximum execution time in milliseconds. Default is None. + success_message: A message to look for in the output of the command to determine if successful or not. process_name (str, optional): A name for the process. If `timeout` is not provided, can be used to terminate the process. force (bool, optional): Whether to execute the command without confirmation. Default is False. @@ -119,8 +121,8 @@ def execute_command(project, command, timeout=None, process_name: str = None, fo Returns: cli_response (str): The command output or: '', 'DONE' if user answered 'no' or 'skip' - llm_response (str): The response from the agent. - TODO: this seems to be 'DONE' (no or skip) or None + llm_response (str): 'DONE' if 'no', 'skip' or `success_message` matched. + Otherwise `None` - caller should send `cli_response` to LLM exit_code (int): The exit code of the process. """ if timeout is not None: @@ -166,6 +168,7 @@ def execute_command(project, command, timeout=None, process_name: str = None, fo return command_run.cli_response, None, None return_value = None + was_success = None q_stderr = queue.Queue() q = queue.Queue() @@ -189,9 +192,9 @@ def execute_command(project, command, timeout=None, process_name: str = None, fo try: while True: elapsed_time = time.time() - start_time - if timeout is not None: - # TODO: print to IPC using a different message type so VS Code can ignore it or update the previous value - print(white_bold(f'\rt: {round(elapsed_time * 1000)}ms : '), end='', flush=True) + # if timeout is not None: + # # TODO: print to IPC using a different message type so VS Code can ignore it or update the previous value + # print(white_bold(f'\rt: {round(elapsed_time * 1000)}ms : '), end='', flush=True) # Check if process has finished if process.poll() is not None: @@ -207,6 +210,10 @@ def execute_command(project, command, timeout=None, process_name: str = None, fo # If timeout is reached, kill the process if timeout is not None and elapsed_time * 1000 > timeout: + if process_name is not None: + logger.info(f'Process "{process_name}" running after timeout as pid: {process.pid}') + break + raise TimeoutError("Command exceeded the specified timeout.") # os.killpg(process.pid, signal.SIGKILL) # break @@ -220,6 +227,10 @@ def execute_command(project, command, timeout=None, process_name: str = None, fo output += line print(green('CLI OUTPUT:') + line, end='') logger.info('CLI OUTPUT: ' + line) + if success_message is not None and success_message in line: + logger.info('Success message found: %s', success_message) + was_success = True + break # Read stderr try: @@ -231,10 +242,6 @@ def execute_command(project, command, timeout=None, process_name: str = None, fo stderr_output += stderr_line print(red('CLI ERROR:') + stderr_line, end='') # Print with different color for distinction logger.error('CLI ERROR: ' + stderr_line) - - if process_name is not None: - logger.info(f'Process "{process_name}" running as pid: {process.pid}') - break except (KeyboardInterrupt, TimeoutError) as e: interrupted = True @@ -245,6 +252,7 @@ def execute_command(project, command, timeout=None, process_name: str = None, fo print('\nTimeout detected. Stopping command execution...') logger.warn('Timeout detected. Stopping command execution...') + was_success = False terminate_process(process.pid) elapsed_time = time.time() - start_time @@ -262,7 +270,7 @@ def execute_command(project, command, timeout=None, process_name: str = None, fo save_command_run(project, command, return_value) - return return_value, None, process.returncode + return return_value, 'DONE' if was_success else None, process.returncode def build_directory_tree(path, prefix="", ignore=None, is_last=False, files=None, add_descriptions=False): @@ -331,6 +339,7 @@ def execute_command_and_check_cli_response(command, timeout, convo): def run_command_until_success(convo, command, timeout: Union[int, None], process_name: Union[str, None] = None, + success_message=None, additional_message=None, force=False, return_cli_response=False, @@ -344,6 +353,7 @@ def run_command_until_success(convo, command, timeout (int): The maximum execution time in milliseconds. process_name: A name for the process. If `timeout` is not provided, can be used to terminate the process. + success_message: A message to look for in the output of the command to determine if successful or not. additional_message (str, optional): Additional message to include in the response. force (bool, optional): Whether to execute the command without confirmation. Default is False. return_cli_response (bool, optional): If True, may raise TooDeepRecursionError(cli_response) @@ -352,6 +362,7 @@ def run_command_until_success(convo, command, cli_response, response, exit_code = execute_command(convo.agent.project, command, timeout=timeout, + success_message=success_message, process_name=process_name, force=force) From c82d4720effb20338615751e6900e163e57b5c86 Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Mon, 9 Oct 2023 19:24:12 +1100 Subject: [PATCH 06/21] WIP - trying to clean & fix `get_full_file_path()` --- pilot/helpers/Project.py | 48 +++++++++++------------------------ pilot/helpers/test_Project.py | 3 ++- 2 files changed, 17 insertions(+), 34 deletions(-) diff --git a/pilot/helpers/Project.py b/pilot/helpers/Project.py index a416e3bbc..fb170daa7 100644 --- a/pilot/helpers/Project.py +++ b/pilot/helpers/Project.py @@ -1,5 +1,6 @@ import json import os +import re from typing import Tuple from utils.style import yellow_bold, cyan, white_bold from const.common import IGNORE_FOLDERS, STEPS @@ -191,16 +192,17 @@ def get_files(self, files): list: A list of files with content. """ files_with_content = [] - for file in files: + for file_path in files: # TODO this is a hack, fix it try: - relative_path, full_path = self.get_full_file_path('', file) + name = os.path.basename(file_path) + relative_path, full_path = self.get_full_file_path(file_path, name) file_content = open(full_path, 'r').read() except: file_content = '' files_with_content.append({ - "path": file, + "path": file_path, "content": file_content }) return files_with_content @@ -212,27 +214,27 @@ def save_file(self, data): Args: data: { name: 'hello.py', path: 'path/to/hello.py', content: 'print("Hello!")' } """ + name = data['name'] if 'name' in data else os.path.basename(data['path']) + path = data['path'] if 'path' in data else name + # TODO fix this in prompts - if 'path' not in data: - data['path'] = data['name'] + if not path.endswith(name): + # if not re.match(r'^/|~|\w+:', path): - if 'name' not in data or data['name'] == '': - data['name'] = os.path.basename(data['path']) - elif not data['path'].endswith(data['name']): if data['path'] == '': data['path'] = data['name'] else: data['path'] = data['path'] + '/' + data['name'] # TODO END - data['path'], data['full_path'] = self.get_full_file_path(data['path'], data['name']) - update_file(data['full_path'], data['content']) + path, full_path = self.get_full_file_path(path, name) + update_file(full_path, data['content']) - (File.insert(app=self.app, path=data['path'], name=data['name'], full_path=data['full_path']) + (File.insert(app=self.app, path=path, name=name, full_path=full_path) .on_conflict( conflict_target=[File.app, File.name, File.path], preserve=[], - update={'name': data['name'], 'path': data['path'], 'full_path': data['full_path']}) + update={'name': name, 'path': path, 'full_path': full_path}) .execute()) def get_full_file_path(self, file_path: str, file_name: str) -> Tuple[str, str]: @@ -240,33 +242,24 @@ def get_full_file_path(self, file_path: str, file_name: str) -> Tuple[str, str]: # WINDOWS are_windows_paths = '\\' in file_path or '\\' in file_name or '\\' in self.root_path if are_windows_paths: - file_name = file_name.replace('\\', '/') file_path = file_path.replace('\\', '/') # END WINDOWS # Universal modifications file_path = file_path.replace('~', '') - file_name = file_name.replace('~', '') file_path = file_path.replace(self.root_path, '') - file_name = file_name.replace(self.root_path, '') if '.' not in file_path and not file_path.endswith('/'): file_path += '/' - if '.' not in file_name and not file_name.endswith('/'): - file_name += '/' if '/' in file_path and not file_path.startswith('/'): file_path = '/' + file_path - if '/' in file_name and not file_name.startswith('/'): - file_name = '/' + file_name # END Universal modifications head_path, tail_path = os.path.split(file_path) - head_name, tail_name = os.path.split(file_name) final_file_path = head_path if head_path != '' else head_name - final_file_name = tail_name if tail_name != '' else tail_path if head_path in head_name: final_file_path = head_name @@ -280,18 +273,7 @@ def get_full_file_path(self, file_path: str, file_name: str) -> Tuple[str, str]: if final_file_path == '': final_file_path = '/' - final_absolute_path = self.root_path + final_file_path + '/' + final_file_name - - if '//' in final_absolute_path: - final_absolute_path = final_absolute_path.replace('//', '/') - if '//' in final_file_path: - final_file_path = final_file_path.replace('//', '/') - - # WINDOWS - if are_windows_paths: - final_file_path = final_file_path.replace('/', '\\') - final_absolute_path = final_absolute_path.replace('/', '\\') - # END WINDOWS + final_absolute_path = self.root_path + final_file_path + '/' + file_name return final_file_path, final_absolute_path diff --git a/pilot/helpers/test_Project.py b/pilot/helpers/test_Project.py index 4ebc8c121..bec940d1d 100644 --- a/pilot/helpers/test_Project.py +++ b/pilot/helpers/test_Project.py @@ -22,6 +22,7 @@ def create_project(): @pytest.mark.parametrize('test_data', [ {'name': 'package.json', 'path': 'package.json', 'saved_to': '/temp/gpt-pilot-test/package.json'}, {'name': 'package.json', 'path': '', 'saved_to': '/temp/gpt-pilot-test/package.json'}, + {'name': 'package.json', 'path': '/', 'saved_to': '/temp/gpt-pilot-test/package.json'}, # observed scenario # {'name': 'Dockerfile', 'path': None, 'saved_to': '/temp/gpt-pilot-test/Dockerfile'}, {'name': None, 'path': 'public/index.html', 'saved_to': '/temp/gpt-pilot-test/public/index.html'}, {'name': '', 'path': 'public/index.html', 'saved_to': '/temp/gpt-pilot-test/public/index.html'}, @@ -32,7 +33,7 @@ def create_project(): # {'name': '.gitconfig', 'path': '~/.gitconfig', 'saved_to': '~/.gitconfig'}, # {'name': 'gpt-pilot.log', 'path': '/temp/gpt-pilot.log', 'saved_to': '/temp/gpt-pilot.log'}, ], ids=[ - 'name == path', 'empty path', + 'name == path', 'empty path', 'slash path', # 'None path', 'None name', 'empty name', # 'None path absolute file', 'home path', 'home path same name', 'absolute path with name' From 69ba6b431e0fdb3906e586347090280f039e1b0f Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Tue, 10 Oct 2023 10:43:42 +1100 Subject: [PATCH 07/21] refatored and tested `get_full_file_path()` --- .github/workflows/ci.yml | 40 +++- pilot/helpers/Project.py | 58 ++---- pilot/helpers/test_Project.py | 374 +++++++++++++++++++++++++--------- 3 files changed, 335 insertions(+), 137 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ba3308e57..27e3172b9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,10 +7,9 @@ on: pull_request: branches: - main - - debugging_ipc jobs: - build: + Ubuntu: runs-on: ubuntu-latest strategy: matrix: @@ -46,3 +45,40 @@ jobs: pip install pytest cd pilot PYTHONPATH=. pytest -m "not slow and not uses_tokens and not ux_test" + + Windows: + runs-on: windows-latest + strategy: + matrix: + # 3.10 - 04 Oct 2021 + # 3.11 - 24 Oct 2022 + python-version: [ '3.9', '3.10', '3.11', '3.12' ] + + steps: + - uses: actions/checkout@v4 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + cache: 'pip' + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + + - name: Lint + run: | + pip install flake8 ruff + flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics + # stop the build if there are Python syntax errors or undefined names + ruff --format=github --select=E9,F63,F7,F82 --target-version=py37 . + # default set of ruff rules with GitHub Annotations + #ruff --format=github --target-version=py37 --ignore=F401,E501 . + + - name: Run tests + run: | + pip install pytest + cd pilot + PYTHONPATH=. pytest -m "not slow and not uses_tokens and not ux_test" diff --git a/pilot/helpers/Project.py b/pilot/helpers/Project.py index fb170daa7..30c9eab63 100644 --- a/pilot/helpers/Project.py +++ b/pilot/helpers/Project.py @@ -214,19 +214,9 @@ def save_file(self, data): Args: data: { name: 'hello.py', path: 'path/to/hello.py', content: 'print("Hello!")' } """ - name = data['name'] if 'name' in data else os.path.basename(data['path']) + name = data['name'] if 'name' in data and data['name'] != '' else os.path.basename(data['path']) path = data['path'] if 'path' in data else name - # TODO fix this in prompts - if not path.endswith(name): - # if not re.match(r'^/|~|\w+:', path): - - if data['path'] == '': - data['path'] = data['name'] - else: - data['path'] = data['path'] + '/' + data['name'] - # TODO END - path, full_path = self.get_full_file_path(path, name) update_file(full_path, data['content']) @@ -238,44 +228,30 @@ def save_file(self, data): .execute()) def get_full_file_path(self, file_path: str, file_name: str) -> Tuple[str, str]: + file_name = os.path.basename(file_name) + + if file_path.startswith(self.root_path): + file_path = file_path.replace(self.root_path, '') - # WINDOWS are_windows_paths = '\\' in file_path or '\\' in file_name or '\\' in self.root_path if are_windows_paths: file_path = file_path.replace('\\', '/') - # END WINDOWS - - # Universal modifications - file_path = file_path.replace('~', '') - - file_path = file_path.replace(self.root_path, '') - if '.' not in file_path and not file_path.endswith('/'): - file_path += '/' - - if '/' in file_path and not file_path.startswith('/'): - file_path = '/' + file_path - # END Universal modifications - - head_path, tail_path = os.path.split(file_path) - - final_file_path = head_path if head_path != '' else head_name - - if head_path in head_name: - final_file_path = head_name - elif final_file_path != head_name: - if head_name not in head_path and head_path not in head_name: - if '.' in file_path: - final_file_path = head_name + head_path - else: - final_file_path = head_path + head_name + # Force all paths to be relative to the workspace + file_path = re.sub(r'^(\w+:/|[/~.]+)', '', file_path, 1) - if final_file_path == '': - final_file_path = '/' + # file_path should not include the file name + if file_path == file_name: + file_path = '' + elif file_path.endswith('/' + file_name): + file_path = file_path.replace('/' + file_name, '') + elif file_path.endswith('/'): + file_path = file_path[:-1] - final_absolute_path = self.root_path + final_file_path + '/' + file_name + absolute_path = self.root_path + '/' + file_name if file_path == '' \ + else self.root_path + '/' + file_path + '/' + file_name - return final_file_path, final_absolute_path + return file_path, absolute_path def save_files_snapshot(self, development_step_id): files = get_files_content(self.root_path, ignore=IGNORE_FOLDERS) diff --git a/pilot/helpers/test_Project.py b/pilot/helpers/test_Project.py index bec940d1d..fdb6e5701 100644 --- a/pilot/helpers/test_Project.py +++ b/pilot/helpers/test_Project.py @@ -19,97 +19,283 @@ def create_project(): return project -@pytest.mark.parametrize('test_data', [ - {'name': 'package.json', 'path': 'package.json', 'saved_to': '/temp/gpt-pilot-test/package.json'}, - {'name': 'package.json', 'path': '', 'saved_to': '/temp/gpt-pilot-test/package.json'}, - {'name': 'package.json', 'path': '/', 'saved_to': '/temp/gpt-pilot-test/package.json'}, # observed scenario - # {'name': 'Dockerfile', 'path': None, 'saved_to': '/temp/gpt-pilot-test/Dockerfile'}, - {'name': None, 'path': 'public/index.html', 'saved_to': '/temp/gpt-pilot-test/public/index.html'}, - {'name': '', 'path': 'public/index.html', 'saved_to': '/temp/gpt-pilot-test/public/index.html'}, - - # TODO: Treatment of paths outside of the project workspace - https://github.com/Pythagora-io/gpt-pilot/issues/129 - # {'name': '/etc/hosts', 'path': None, 'saved_to': '/etc/hosts'}, - # {'name': '.gitconfig', 'path': '~', 'saved_to': '~/.gitconfig'}, - # {'name': '.gitconfig', 'path': '~/.gitconfig', 'saved_to': '~/.gitconfig'}, - # {'name': 'gpt-pilot.log', 'path': '/temp/gpt-pilot.log', 'saved_to': '/temp/gpt-pilot.log'}, -], ids=[ - 'name == path', 'empty path', 'slash path', - # 'None path', - 'None name', 'empty name', - # 'None path absolute file', 'home path', 'home path same name', 'absolute path with name' -]) -@patch('helpers.Project.update_file') -@patch('helpers.Project.File') -def test_save_file(mock_file_insert, mock_update_file, test_data): - # Given - data = {'content': 'Hello World!'} - if test_data['name'] is not None: - data['name'] = test_data['name'] - if test_data['path'] is not None: - data['path'] = test_data['path'] - - project = create_project() - - # When - project.save_file(data) - - # Then assert that update_file with the correct path - expected_saved_to = test_data['saved_to'] - mock_update_file.assert_called_once_with(expected_saved_to, 'Hello World!') - - # Also assert that File.insert was called with the expected arguments - # expected_file_data = {'app': project.app, 'path': test_data['path'], 'name': test_data['name'], - # 'full_path': expected_saved_to} - # mock_file_insert.assert_called_once_with(app=project.app, **expected_file_data, - # **{'name': test_data['name'], 'path': test_data['path'], - # 'full_path': expected_saved_to}) - - -@pytest.mark.parametrize('file_path, file_name, expected', [ - ('file.txt', 'file.txt', '/temp/gpt-pilot-test/file.txt'), - ('', 'file.txt', '/temp/gpt-pilot-test/file.txt'), - ('path/', 'file.txt', '/temp/gpt-pilot-test/path/file.txt'), - ('path/to/', 'file.txt', '/temp/gpt-pilot-test/path/to/file.txt'), - ('path/to/file.txt', 'file.txt', '/temp/gpt-pilot-test/path/to/file.txt'), - ('./path/to/file.txt', 'file.txt', '/temp/gpt-pilot-test/./path/to/file.txt'), # ideally result would not have `./` -]) -def test_get_full_path(file_path, file_name, expected): - # Given - project = create_project() - - # When - relative_path, absolute_path = project.get_full_file_path(file_path, file_name) - - # Then - assert absolute_path == expected - - -@pytest.mark.skip(reason="Handling of absolute paths will be revisited in #29") -@pytest.mark.parametrize('file_path, file_name, expected', [ - ('/file.txt', 'file.txt', '/file.txt'), - ('/path/to/file.txt', 'file.txt', '/path/to/file.txt'), - # Only passes on Windows? ('C:\\path\\to\\file.txt', 'file.txt', 'C:\\path\\to/file.txt'), - ('~/path/to/file.txt', 'file.txt', '~/path/to/file.txt'), -]) -def test_get_full_path_absolute(file_path, file_name, expected): - # Given - project = create_project() - - # When - relative_path, absolute_path = project.get_full_file_path(file_path, file_name) - - # Then - assert absolute_path == expected - - -# This is known to fail and should be avoided -# def test_get_full_file_path_error(): -# # Given -# file_path = 'path/to/file/' -# file_name = '' -# -# # When -# full_path = project.get_full_file_path(file_path, file_name) -# -# # Then -# assert full_path == '/temp/gpt-pilot-test/path/to/file/' +class TestProject: + def test_save_file_permutations(self): + project = create_project() + project.root_path = '/Users/zvonimirsabljic/Development/copilot/pilot' + + values = [ + "", + "server.js", + "~/", + "~/server.js", + "/", + "/server.js", + "/Users/zvonimirsabljic/Development/copilot/pilot", + "/Users/zvonimirsabljic/Development/copilot/pilot/", + "/Users/zvonimirsabljic/Development/copilot/pilot/server.js" + ] + + for file_name in values: + if file_name.endswith('server.js'): + for file_path in values: + out_file_path, absolute_path = project.get_full_file_path(file_path, file_name) + # print(f"file_path: {file_path} -> {out_file_path}, \tabsolute_path: {absolute_path}") + assert out_file_path == '', f'file_path: {file_path}, file_name: {file_name}' + # if absolute_path != '/Users/zvonimirsabljic/Development/copilot/pilot/server.js': + # print(f'file_path: {file_path}, file_name: {file_name}') + assert absolute_path == '/Users/zvonimirsabljic/Development/copilot/pilot/server.js',\ + f'file_path: {file_path}, file_name: {file_name}' + + def test_save_file_permutations_deeper(self): + project = create_project() + project.root_path = '/Users/zvonimirsabljic/Development/copilot/pilot' + + values = [ + "", + "/", + "~/", + "folder1/folder2/server.js", + "/folder1/folder2/server.js", + "~/folder1/folder2/server.js", + "/Users/zvonimirsabljic/Development/copilot/pilot/folder1/folder2/server.js", + + "folder1/", + "/folder1/", + "~/folder1/", + "/Users/zvonimirsabljic/Development/copilot/pilot/folder1/", + + "folder1", + "/folder1", + "~/folder1", + "/Users/zvonimirsabljic/Development/copilot/pilot/folder1", + + "folder1/folder2/", + "/folder1/folder2/", + "~/folder1/folder2/", + "/Users/zvonimirsabljic/Development/copilot/pilot/folder1/folder2/", + + "folder1/folder2", + "/folder1/folder2", + "~/folder1/folder2", + "/Users/zvonimirsabljic/Development/copilot/pilot/folder1/folder2", + + "server.js", + "/server.js", + "~/server.js", + + "folder2/server.js", + "/folder2/server.js", + "~/folder2/server.js", + "/Users/zvonimirsabljic/Development/copilot/pilot/folder2/server.js", + ] + # values = ['', 'folder1/folder2/server.js'] + + for file_name in values: + if file_name.endswith('server.js'): + for file_path in values: + expected_path = '' + if 'folder1' in file_path: + if 'folder1/folder2' in file_path: + expected_path = 'folder1/folder2' + else: + expected_path = 'folder1' + elif 'folder2' in file_path: + expected_path = 'folder2' + + expected_absolute_path = project.root_path + \ + ('' if expected_path == '' else '/' + expected_path) + '/server.js' + + out_file_path, absolute_path = project.get_full_file_path(file_path, file_name) + # print(f"file_path: {file_path} -> {out_file_path}, \tabsolute_path: {absolute_path}") + assert out_file_path == expected_path, f'file_path: {file_path}, file_name: {file_name}' + # if absolute_path != expected_absolute_path: + # print(f'file_path: {file_path}, file_name: {file_name}') + assert absolute_path == expected_absolute_path, f'file_path: {file_path}, file_name: {file_name}' + + def test_save_file_permutations_windows(self): + project = create_project() + project.root_path = 'C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot' + + values = [ + "", + "server.js", + "~\\", + "~\\server.js", + "C:\\", + "C:\\server.js", + "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot", + "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\", + "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\server.js" + ] + values = ['C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot', 'server.js'] + + for file_name in values: + if file_name.endswith('server.js'): + for file_path in values: + out_file_path, absolute_path = project.get_full_file_path(file_path, file_name) + # print(f"file_path: {file_path} -> {out_file_path}, \tabsolute_path: {absolute_path}") + assert out_file_path == '', f'file_path: {file_path}, file_name: {file_name}' + # if absolute_path != '/Users/zvonimirsabljic/Development/copilot/pilot/server.js': + # print(f'file_path: {file_path}, file_name: {file_name}') + assert absolute_path == project.root_path + '/server.js',\ + f'file_path: {file_path}, file_name: {file_name}' + + def test_save_file_permutations_windows_deeper(self): + project = create_project() + project.root_path = 'C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot' + + values = [ + "", + "C:\\", + "~\\", + "fol der1\\fold er2\\server.js", + "C:\\folder1\\folder2\\server.js", + "~\\folder1\\folder2\\server.js", + "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\folder1\\folder2\\server.js", + + "folder1\\", + "C:\\folder1\\", + "~\\folder1\\", + "\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\folder1\\", + + "folder1", + "C:\\folder1", + "~\\folder1", + "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\folder1", + + "folder1\\folder2\\", + "C:\\folder1\\folder2\\", + "~\\folder1\\folder2\\", + "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\folder1\\folder2\\", + + "folder1\\folder2", + "C:\\folder1\\folder2", + "~\\folder1\\folder2", + "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\folder1\\folder2", + + "server.js", + "C:\\server.js", + "~\\server.js", + + "folder2\\server.js", + "C:\\folder2\\server.js", + "~\\folder2\\server.js", + "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\folder2\\server.js", + ] + values = ['C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot', 'server.js'] + + for file_name in values: + if file_name.endswith('server.js'): + for file_path in values: + expected_path = '' + if 'folder1' in file_path: + if 'folder1/folder2' in file_path: + expected_path = 'folder1/folder2' + else: + expected_path = 'folder1' + elif 'folder2' in file_path: + expected_path = 'folder2' + + expected_absolute_path = project.root_path + \ + ('' if expected_path == '' else '/' + expected_path) + '/server.js' + + out_file_path, absolute_path = project.get_full_file_path(file_path, file_name) + # print(f"file_path: {file_path} -> {out_file_path}, \tabsolute_path: {absolute_path}") + assert out_file_path == expected_path, f'file_path: {file_path}, file_name: {file_name}' + # if absolute_path != expected_absolute_path: + # print(f'file_path: {file_path}, file_name: {file_name}') + assert absolute_path == expected_absolute_path, f'file_path: {file_path}, file_name: {file_name}' + + @pytest.mark.parametrize('test_data', [ + {'name': 'package.json', 'path': 'package.json', 'saved_to': '/temp/gpt-pilot-test/package.json'}, + {'name': 'package.json', 'path': '', 'saved_to': '/temp/gpt-pilot-test/package.json'}, + {'name': 'package.json', 'path': '/', 'saved_to': '/temp/gpt-pilot-test/package.json'}, # observed scenario + {'name': 'Dockerfile', 'path': None, 'saved_to': '/temp/gpt-pilot-test/Dockerfile'}, + {'name': None, 'path': 'public/index.html', 'saved_to': '/temp/gpt-pilot-test/public/index.html'}, + {'name': '', 'path': 'public/index.html', 'saved_to': '/temp/gpt-pilot-test/public/index.html'}, + + # TODO: Treatment of paths outside of the project workspace - https://github.com/Pythagora-io/gpt-pilot/issues/129 + # {'name': '/etc/hosts', 'path': None, 'saved_to': '/etc/hosts'}, + # {'name': '.gitconfig', 'path': '~', 'saved_to': '~/.gitconfig'}, + # {'name': '.gitconfig', 'path': '~/.gitconfig', 'saved_to': '~/.gitconfig'}, + # {'name': 'gpt-pilot.log', 'path': '/temp/gpt-pilot.log', 'saved_to': '/temp/gpt-pilot.log'}, + # ], ids=[ + # 'name == path', 'empty path', 'slash path', + # # 'None path', + # 'None name', 'empty name', + # 'None path absolute file', 'home path', 'home path same name', 'absolute path with name' + ]) + @patch('helpers.Project.update_file') + @patch('helpers.Project.File') + def test_save_file(self, mock_file_insert, mock_update_file, test_data): + # Given + data = {'content': 'Hello World!'} + if test_data['name'] is not None: + data['name'] = test_data['name'] + if test_data['path'] is not None: + data['path'] = test_data['path'] + + project = create_project() + + # When + project.save_file(data) + + # Then assert that update_file with the correct path + expected_saved_to = test_data['saved_to'] + mock_update_file.assert_called_once_with(expected_saved_to, 'Hello World!') + + # Also assert that File.insert was called with the expected arguments + # expected_file_data = {'app': project.app, 'path': test_data['path'], 'name': test_data['name'], + # 'full_path': expected_saved_to} + # mock_file_insert.assert_called_once_with(app=project.app, **expected_file_data, + # **{'name': test_data['name'], 'path': test_data['path'], + # 'full_path': expected_saved_to}) + + @pytest.mark.parametrize('file_path, file_name, expected', [ + ('file.txt', 'file.txt', '/temp/gpt-pilot-test/file.txt'), + ('', 'file.txt', '/temp/gpt-pilot-test/file.txt'), + ('path/', 'file.txt', '/temp/gpt-pilot-test/path/file.txt'), + ('path/to/', 'file.txt', '/temp/gpt-pilot-test/path/to/file.txt'), + ('path/to/file.txt', 'file.txt', '/temp/gpt-pilot-test/path/to/file.txt'), + ('./path/to/file.txt', 'file.txt', '/temp/gpt-pilot-test/path/to/file.txt'), + ]) + def test_get_full_path(self, file_path, file_name, expected): + # Given + project = create_project() + + # When + relative_path, absolute_path = project.get_full_file_path(file_path, file_name) + + # Then + assert absolute_path == expected + + @pytest.mark.skip(reason="Handling of absolute paths will be revisited in #29") + @pytest.mark.parametrize('file_path, file_name, expected', [ + ('/file.txt', 'file.txt', '/file.txt'), + ('/path/to/file.txt', 'file.txt', '/path/to/file.txt'), + # Only passes on Windows? ('C:\\path\\to\\file.txt', 'file.txt', 'C:\\path\\to/file.txt'), + ('~/path/to/file.txt', 'file.txt', '~/path/to/file.txt'), + ]) + def test_get_full_path_absolute(self, file_path, file_name, expected): + # Given + project = create_project() + + # When + relative_path, absolute_path = project.get_full_file_path(file_path, file_name) + + # Then + assert absolute_path == expected + + # # This is known to fail and should be avoided + # def test_get_full_file_path_error(self): + # # Given + # project = create_project() + # file_path = 'path/to/file/' + # file_name = '' + # + # # When + # relative_path, full_path = project.get_full_file_path(file_path, file_name) + # + # # Then + # assert full_path == '/temp/gpt-pilot-test/path/to/file/' From 44fa7bbd5e211b33aea4d121ada5f3791f4c32c1 Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Tue, 10 Oct 2023 10:52:05 +1100 Subject: [PATCH 08/21] fix Windows CI --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 27e3172b9..836e8c5c2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -81,4 +81,4 @@ jobs: run: | pip install pytest cd pilot - PYTHONPATH=. pytest -m "not slow and not uses_tokens and not ux_test" + pytest -m "not slow and not uses_tokens and not ux_test" From db30809511ebd7ef56548205f4b16bd0de479e20 Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Tue, 10 Oct 2023 11:17:19 +1100 Subject: [PATCH 09/21] fixed env --- .github/workflows/ci.yml | 44 +++++----------------------------------- 1 file changed, 5 insertions(+), 39 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 836e8c5c2..1aac9477d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,12 +10,13 @@ on: jobs: Ubuntu: - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} strategy: matrix: # 3.10 - 04 Oct 2021 # 3.11 - 24 Oct 2022 python-version: ['3.9', '3.10', '3.11', '3.12'] + os: [ubuntu-latest, macos-latest, windows-latest] steps: - uses: actions/checkout@v4 @@ -41,44 +42,9 @@ jobs: #ruff --format=github --target-version=py37 --ignore=F401,E501 . - name: Run tests + env: + PYTHONPATH: . run: | pip install pytest cd pilot - PYTHONPATH=. pytest -m "not slow and not uses_tokens and not ux_test" - - Windows: - runs-on: windows-latest - strategy: - matrix: - # 3.10 - 04 Oct 2021 - # 3.11 - 24 Oct 2022 - python-version: [ '3.9', '3.10', '3.11', '3.12' ] - - steps: - - uses: actions/checkout@v4 - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v4 - with: - python-version: ${{ matrix.python-version }} - cache: 'pip' - - - name: Install dependencies - run: | - python -m pip install --upgrade pip - pip install -r requirements.txt - - - name: Lint - run: | - pip install flake8 ruff - flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics - # stop the build if there are Python syntax errors or undefined names - ruff --format=github --select=E9,F63,F7,F82 --target-version=py37 . - # default set of ruff rules with GitHub Annotations - #ruff --format=github --target-version=py37 --ignore=F401,E501 . - - - name: Run tests - run: | - pip install pytest - cd pilot - pytest -m "not slow and not uses_tokens and not ux_test" + pytest -m "not slow and not uses_tokens and not ux_test" From 1179675c6b5c8bb640138d9aec4c3d77bfe25ec3 Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Tue, 10 Oct 2023 11:18:30 +1100 Subject: [PATCH 10/21] Rename "Test" --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1aac9477d..9fd7395dc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,7 +9,7 @@ on: - main jobs: - Ubuntu: + Test: runs-on: ${{ matrix.os }} strategy: matrix: From 08280d4371498811ee304ed1ec4da1c1d844657a Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Tue, 10 Oct 2023 11:42:41 +1100 Subject: [PATCH 11/21] Skip Python 3.12 on Windows for now > LINK : fatal error LNK1181: cannot open input file 'libpq.lib' Maybe related: https://github.com/psycopg/psycopg2/issues/1628 --- .github/workflows/ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9fd7395dc..5da8b777a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,6 +17,11 @@ jobs: # 3.11 - 24 Oct 2022 python-version: ['3.9', '3.10', '3.11', '3.12'] os: [ubuntu-latest, macos-latest, windows-latest] + exclude: + # LINK : fatal error LNK1181: cannot open input file 'libpq.lib' + # Maybe related: https://github.com/psycopg/psycopg2/issues/1628 + - os: windows-latest + python-version: '3.12' steps: - uses: actions/checkout@v4 From 33efb72d7aa61bfa9ba0a99fa23b28a5d9d04324 Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Tue, 10 Oct 2023 12:39:54 +1100 Subject: [PATCH 12/21] flush input before asking a new question. --- pilot/utils/questionary.py | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/pilot/utils/questionary.py b/pilot/utils/questionary.py index b9877b105..c259e2354 100644 --- a/pilot/utils/questionary.py +++ b/pilot/utils/questionary.py @@ -1,7 +1,10 @@ -from prompt_toolkit.styles import Style import questionary -from utils.style import yellow_bold import re +import sys +import termios +from prompt_toolkit.styles import Style +from utils.style import yellow_bold + from database.database import save_user_input, get_saved_user_input custom_style = Style.from_dict({ @@ -38,7 +41,8 @@ def styled_text(project, question, ignore_user_input_count=False, style=None): config = { 'style': style if style is not None else custom_style, } - question = remove_ansi_codes(question) # Colorama and questionary are not compatible and styling doesn't work + question = remove_ansi_codes(question) # Colorama and questionary are not compatible and styling doesn't work + flush_input() response = questionary.text(question, **config).unsafe_ask() # .ask() is included here else: response = print(question, type='user_input_request') @@ -56,3 +60,18 @@ def get_user_feedback(): 'style': custom_style, } return questionary.text("How did GPT Pilot do? Were you able to create any app that works? Please write any feedback you have or just press ENTER to exit: ", **config).unsafe_ask() + + +def flush_input(): + """Flush the input buffer, discarding all that's in the buffer.""" + try: + # For Unix-like systems + termios.tcflush(sys.stdin, termios.TCIOFLUSH) + except Exception: + # For Windows systems + try: + import msvcrt + while msvcrt.kbhit(): + msvcrt.getch() + except ImportError: + pass From 3f62a60d9a95daae048f532eaa140cb199fb2431 Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Tue, 10 Oct 2023 12:43:36 +1100 Subject: [PATCH 13/21] tidy up --- pilot/utils/questionary.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pilot/utils/questionary.py b/pilot/utils/questionary.py index c259e2354..38b929d58 100644 --- a/pilot/utils/questionary.py +++ b/pilot/utils/questionary.py @@ -22,7 +22,7 @@ def remove_ansi_codes(s: str) -> str: def styled_select(*args, **kwargs): - kwargs["style"] = custom_style # Set style here + kwargs["style"] = custom_style return questionary.select(*args, **kwargs).unsafe_ask() # .ask() is included here @@ -59,7 +59,8 @@ def get_user_feedback(): config = { 'style': custom_style, } - return questionary.text("How did GPT Pilot do? Were you able to create any app that works? Please write any feedback you have or just press ENTER to exit: ", **config).unsafe_ask() + return questionary.text('How did GPT Pilot do? Were you able to create any app that works? ' + 'Please write any feedback you have or just press ENTER to exit: ', **config).unsafe_ask() def flush_input(): @@ -67,11 +68,11 @@ def flush_input(): try: # For Unix-like systems termios.tcflush(sys.stdin, termios.TCIOFLUSH) - except Exception: + except (ImportError, termios.error): # For Windows systems try: import msvcrt while msvcrt.kbhit(): msvcrt.getch() - except ImportError: + except (ImportError, OSError): pass From d1e313fe83e5d707325477af61edb0c33281f27b Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Tue, 10 Oct 2023 14:13:31 +1100 Subject: [PATCH 14/21] Added logging to capture bad JSON --- pilot/helpers/AgentConvo.py | 2 ++ pilot/utils/llm_connection.py | 2 +- pilot/utils/questionary.py | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pilot/helpers/AgentConvo.py b/pilot/helpers/AgentConvo.py index 16397851e..e450de84b 100644 --- a/pilot/helpers/AgentConvo.py +++ b/pilot/helpers/AgentConvo.py @@ -82,6 +82,8 @@ def send_message(self, prompt_path=None, prompt_data=None, function_calls: Funct development_step = save_development_step(self.agent.project, prompt_path, prompt_data, self.messages, response) # TODO handle errors from OpenAI + # It's complicated because calling functions are expecting different types of responses - string or tuple + # https://github.com/Pythagora-io/gpt-pilot/issues/165 & #91 if response == {}: logger.error(f'Aborting with "OpenAI API error happened"') raise Exception("OpenAI API error happened.") diff --git a/pilot/utils/llm_connection.py b/pilot/utils/llm_connection.py index 4e9fcf056..f5b5a005e 100644 --- a/pilot/utils/llm_connection.py +++ b/pilot/utils/llm_connection.py @@ -192,6 +192,7 @@ def wrapper(*args, **kwargs): # or `Expecting value` with `pos` before the end of `e.doc` function_error_count = update_error_count(args) logger.warning('Received invalid character in JSON response from LLM. Asking to retry...') + logger.info(f' received: {e.doc}') set_function_error(args, err_str) if function_error_count < 3: continue @@ -255,7 +256,6 @@ def stream_gpt_completion(data, req_type, project): :param project: NEEDED FOR WRAPPER FUNCTION retry_on_exception :return: {'text': str} or {'function_calls': {'name': str, arguments: '{...}'}} """ - # TODO add type dynamically - this isn't working when connected to the external process try: terminal_width = os.get_terminal_size().columns diff --git a/pilot/utils/questionary.py b/pilot/utils/questionary.py index 38b929d58..32ee89ac4 100644 --- a/pilot/utils/questionary.py +++ b/pilot/utils/questionary.py @@ -1,7 +1,6 @@ import questionary import re import sys -import termios from prompt_toolkit.styles import Style from utils.style import yellow_bold @@ -67,8 +66,9 @@ def flush_input(): """Flush the input buffer, discarding all that's in the buffer.""" try: # For Unix-like systems + import termios termios.tcflush(sys.stdin, termios.TCIOFLUSH) - except (ImportError, termios.error): + except (ImportError, OSError): # For Windows systems try: import msvcrt From 0036cb8fe91f3f5236890d40617505b0da5372cc Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Tue, 10 Oct 2023 20:08:30 +1100 Subject: [PATCH 15/21] added test_Debugger --- pilot/helpers/test_Debugger.py | 84 ++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 pilot/helpers/test_Debugger.py diff --git a/pilot/helpers/test_Debugger.py b/pilot/helpers/test_Debugger.py new file mode 100644 index 000000000..eb7aeb9e2 --- /dev/null +++ b/pilot/helpers/test_Debugger.py @@ -0,0 +1,84 @@ +import builtins +import pytest +from unittest.mock import patch +from dotenv import load_dotenv + +load_dotenv() +from pilot.utils.custom_print import get_custom_print +from pilot.helpers.agents.Developer import Developer +from pilot.helpers.AgentConvo import AgentConvo +from pilot.helpers.Debugger import Debugger +from pilot.helpers.test_Project import create_project +from pilot.test.mock_questionary import MockQuestionary + + +################## NOTE: this test needs to be ran in debug with breakpoints ################## + +@pytest.mark.uses_tokens +@patch('pilot.helpers.AgentConvo.get_saved_development_step') +@patch('pilot.helpers.AgentConvo.save_development_step') +@patch('utils.questionary.get_saved_user_input') +@patch('utils.questionary.save_user_input') +@patch('helpers.cli.get_saved_command_run') +@patch('helpers.cli.run_command') +@patch('helpers.cli.save_command_run') +# @patch('pilot.helpers.cli.execute_command', return_value=('', 'DONE', 0)) +def test_debug( + # mock_execute_command, + mock_save_command, mock_run_command, mock_get_saved_command, + mock_save_input, mock_user_input, mock_save_step, mock_get_saved_step): + # Given + builtins.print, ipc_client_instance = get_custom_print({}) + project = create_project() + project.current_step = 'coding' + developer = Developer(project) + project.developer = developer + convo = AgentConvo(developer) + convo.load_branch = lambda x: None + + debugger = Debugger(developer) + # TODO: mock agent.project.developer.execute_task + + # convo.messages.append() + convo.construct_and_add_message_from_prompt('dev_ops/ran_command.prompt', { + 'cli_response': ''' +stderr: +``` +node:internal/modules/cjs/loader:1080 + throw err; + ^ + +Error: Cannot find module 'mime' +Require stack: +- /workspace/chat_app/node_modules/send/index.js +- /workspace/chat_app/node_modules/express/lib/utils.js +- /workspace/chat_app/node_modules/express/lib/application.js +- /workspace/chat_app/node_modules/express/lib/express.js +- /workspace/chat_app/node_modules/express/index.js +- /workspace/chat_app/server.js + at Module._resolveFilename (node:internal/modules/cjs/loader:1077:15) + at Module._load (node:internal/modules/cjs/loader:922:27) + at Module.require (node:internal/modules/cjs/loader:1143:19) + at require (node:internal/modules/cjs/helpers:121:18) + at Object. (/workspace/chat_app/node_modules/send/index.js:24:12) + at Module._compile (node:internal/modules/cjs/loader:1256:14) + at Module._extensions..js (node:internal/modules/cjs/loader:1310:10) + at Module.load (node:internal/modules/cjs/loader:1119:32) + at Module._load (node:internal/modules/cjs/loader:960:12) +``` +stdout: +``` +> chat_app@1.0.0 start +> node server.js +``` +''' + }) + + mock_questionary = MockQuestionary(['', '']) + + with patch('utils.questionary.questionary', mock_questionary): + # When + result = debugger.debug(convo, command={'command': 'npm run start'}, is_root_task=True) + + # Then + assert result == {'success': True} From 8d437ef1189e4d74bb235abe83595b9ae7c79c87 Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Tue, 10 Oct 2023 20:09:13 +1100 Subject: [PATCH 16/21] refactored flush_input to check OS --- pilot/utils/questionary.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/pilot/utils/questionary.py b/pilot/utils/questionary.py index 32ee89ac4..dd1d4a2f9 100644 --- a/pilot/utils/questionary.py +++ b/pilot/utils/questionary.py @@ -1,3 +1,4 @@ +import platform import questionary import re import sys @@ -65,14 +66,12 @@ def get_user_feedback(): def flush_input(): """Flush the input buffer, discarding all that's in the buffer.""" try: - # For Unix-like systems - import termios - termios.tcflush(sys.stdin, termios.TCIOFLUSH) - except (ImportError, OSError): - # For Windows systems - try: + if platform.system() == 'Windows': import msvcrt while msvcrt.kbhit(): msvcrt.getch() - except (ImportError, OSError): - pass + else: + import termios + termios.tcflush(sys.stdin, termios.TCIOFLUSH) + except (ImportError, OSError): + pass From cd44655fb8455fff4873e6da68a0f6855e416790 Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Tue, 10 Oct 2023 21:16:42 +1100 Subject: [PATCH 17/21] test Docker issue #93 --- .github/workflows/ci.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5da8b777a..a5080658d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,6 +9,19 @@ on: - main jobs: + Docker: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Build the Docker image + run: docker compose build + - name: Run the Docker image + run: docker compose up gpt-pilot -d + - name: Wait for the Docker image to start + run: docker ps + - name: Stop the Docker image + run: docker compose down + Test: runs-on: ${{ matrix.os }} strategy: From 5aa84f796b7b6e70b62cb94659d4dce48698557c Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Wed, 11 Oct 2023 10:52:04 +1100 Subject: [PATCH 18/21] moved `get full file path()` changes to separate branch/PR --- pilot/helpers/Project.py | 96 ++++++--- pilot/helpers/test_Project.py | 373 +++++++++------------------------- 2 files changed, 162 insertions(+), 307 deletions(-) diff --git a/pilot/helpers/Project.py b/pilot/helpers/Project.py index 30c9eab63..a416e3bbc 100644 --- a/pilot/helpers/Project.py +++ b/pilot/helpers/Project.py @@ -1,6 +1,5 @@ import json import os -import re from typing import Tuple from utils.style import yellow_bold, cyan, white_bold from const.common import IGNORE_FOLDERS, STEPS @@ -192,17 +191,16 @@ def get_files(self, files): list: A list of files with content. """ files_with_content = [] - for file_path in files: + for file in files: # TODO this is a hack, fix it try: - name = os.path.basename(file_path) - relative_path, full_path = self.get_full_file_path(file_path, name) + relative_path, full_path = self.get_full_file_path('', file) file_content = open(full_path, 'r').read() except: file_content = '' files_with_content.append({ - "path": file_path, + "path": file, "content": file_content }) return files_with_content @@ -214,44 +212,88 @@ def save_file(self, data): Args: data: { name: 'hello.py', path: 'path/to/hello.py', content: 'print("Hello!")' } """ - name = data['name'] if 'name' in data and data['name'] != '' else os.path.basename(data['path']) - path = data['path'] if 'path' in data else name + # TODO fix this in prompts + if 'path' not in data: + data['path'] = data['name'] + + if 'name' not in data or data['name'] == '': + data['name'] = os.path.basename(data['path']) + elif not data['path'].endswith(data['name']): + if data['path'] == '': + data['path'] = data['name'] + else: + data['path'] = data['path'] + '/' + data['name'] + # TODO END - path, full_path = self.get_full_file_path(path, name) - update_file(full_path, data['content']) + data['path'], data['full_path'] = self.get_full_file_path(data['path'], data['name']) + update_file(data['full_path'], data['content']) - (File.insert(app=self.app, path=path, name=name, full_path=full_path) + (File.insert(app=self.app, path=data['path'], name=data['name'], full_path=data['full_path']) .on_conflict( conflict_target=[File.app, File.name, File.path], preserve=[], - update={'name': name, 'path': path, 'full_path': full_path}) + update={'name': data['name'], 'path': data['path'], 'full_path': data['full_path']}) .execute()) def get_full_file_path(self, file_path: str, file_name: str) -> Tuple[str, str]: - file_name = os.path.basename(file_name) - - if file_path.startswith(self.root_path): - file_path = file_path.replace(self.root_path, '') + # WINDOWS are_windows_paths = '\\' in file_path or '\\' in file_name or '\\' in self.root_path if are_windows_paths: + file_name = file_name.replace('\\', '/') file_path = file_path.replace('\\', '/') + # END WINDOWS + + # Universal modifications + file_path = file_path.replace('~', '') + file_name = file_name.replace('~', '') + + file_path = file_path.replace(self.root_path, '') + file_name = file_name.replace(self.root_path, '') + + if '.' not in file_path and not file_path.endswith('/'): + file_path += '/' + if '.' not in file_name and not file_name.endswith('/'): + file_name += '/' + + if '/' in file_path and not file_path.startswith('/'): + file_path = '/' + file_path + if '/' in file_name and not file_name.startswith('/'): + file_name = '/' + file_name + # END Universal modifications + + head_path, tail_path = os.path.split(file_path) + head_name, tail_name = os.path.split(file_name) + + final_file_path = head_path if head_path != '' else head_name + final_file_name = tail_name if tail_name != '' else tail_path + + if head_path in head_name: + final_file_path = head_name + elif final_file_path != head_name: + if head_name not in head_path and head_path not in head_name: + if '.' in file_path: + final_file_path = head_name + head_path + else: + final_file_path = head_path + head_name + + if final_file_path == '': + final_file_path = '/' - # Force all paths to be relative to the workspace - file_path = re.sub(r'^(\w+:/|[/~.]+)', '', file_path, 1) + final_absolute_path = self.root_path + final_file_path + '/' + final_file_name - # file_path should not include the file name - if file_path == file_name: - file_path = '' - elif file_path.endswith('/' + file_name): - file_path = file_path.replace('/' + file_name, '') - elif file_path.endswith('/'): - file_path = file_path[:-1] + if '//' in final_absolute_path: + final_absolute_path = final_absolute_path.replace('//', '/') + if '//' in final_file_path: + final_file_path = final_file_path.replace('//', '/') - absolute_path = self.root_path + '/' + file_name if file_path == '' \ - else self.root_path + '/' + file_path + '/' + file_name + # WINDOWS + if are_windows_paths: + final_file_path = final_file_path.replace('/', '\\') + final_absolute_path = final_absolute_path.replace('/', '\\') + # END WINDOWS - return file_path, absolute_path + return final_file_path, final_absolute_path def save_files_snapshot(self, development_step_id): files = get_files_content(self.root_path, ignore=IGNORE_FOLDERS) diff --git a/pilot/helpers/test_Project.py b/pilot/helpers/test_Project.py index fdb6e5701..4ebc8c121 100644 --- a/pilot/helpers/test_Project.py +++ b/pilot/helpers/test_Project.py @@ -19,283 +19,96 @@ def create_project(): return project -class TestProject: - def test_save_file_permutations(self): - project = create_project() - project.root_path = '/Users/zvonimirsabljic/Development/copilot/pilot' - - values = [ - "", - "server.js", - "~/", - "~/server.js", - "/", - "/server.js", - "/Users/zvonimirsabljic/Development/copilot/pilot", - "/Users/zvonimirsabljic/Development/copilot/pilot/", - "/Users/zvonimirsabljic/Development/copilot/pilot/server.js" - ] - - for file_name in values: - if file_name.endswith('server.js'): - for file_path in values: - out_file_path, absolute_path = project.get_full_file_path(file_path, file_name) - # print(f"file_path: {file_path} -> {out_file_path}, \tabsolute_path: {absolute_path}") - assert out_file_path == '', f'file_path: {file_path}, file_name: {file_name}' - # if absolute_path != '/Users/zvonimirsabljic/Development/copilot/pilot/server.js': - # print(f'file_path: {file_path}, file_name: {file_name}') - assert absolute_path == '/Users/zvonimirsabljic/Development/copilot/pilot/server.js',\ - f'file_path: {file_path}, file_name: {file_name}' - - def test_save_file_permutations_deeper(self): - project = create_project() - project.root_path = '/Users/zvonimirsabljic/Development/copilot/pilot' - - values = [ - "", - "/", - "~/", - "folder1/folder2/server.js", - "/folder1/folder2/server.js", - "~/folder1/folder2/server.js", - "/Users/zvonimirsabljic/Development/copilot/pilot/folder1/folder2/server.js", - - "folder1/", - "/folder1/", - "~/folder1/", - "/Users/zvonimirsabljic/Development/copilot/pilot/folder1/", - - "folder1", - "/folder1", - "~/folder1", - "/Users/zvonimirsabljic/Development/copilot/pilot/folder1", - - "folder1/folder2/", - "/folder1/folder2/", - "~/folder1/folder2/", - "/Users/zvonimirsabljic/Development/copilot/pilot/folder1/folder2/", - - "folder1/folder2", - "/folder1/folder2", - "~/folder1/folder2", - "/Users/zvonimirsabljic/Development/copilot/pilot/folder1/folder2", - - "server.js", - "/server.js", - "~/server.js", - - "folder2/server.js", - "/folder2/server.js", - "~/folder2/server.js", - "/Users/zvonimirsabljic/Development/copilot/pilot/folder2/server.js", - ] - # values = ['', 'folder1/folder2/server.js'] - - for file_name in values: - if file_name.endswith('server.js'): - for file_path in values: - expected_path = '' - if 'folder1' in file_path: - if 'folder1/folder2' in file_path: - expected_path = 'folder1/folder2' - else: - expected_path = 'folder1' - elif 'folder2' in file_path: - expected_path = 'folder2' - - expected_absolute_path = project.root_path + \ - ('' if expected_path == '' else '/' + expected_path) + '/server.js' - - out_file_path, absolute_path = project.get_full_file_path(file_path, file_name) - # print(f"file_path: {file_path} -> {out_file_path}, \tabsolute_path: {absolute_path}") - assert out_file_path == expected_path, f'file_path: {file_path}, file_name: {file_name}' - # if absolute_path != expected_absolute_path: - # print(f'file_path: {file_path}, file_name: {file_name}') - assert absolute_path == expected_absolute_path, f'file_path: {file_path}, file_name: {file_name}' - - def test_save_file_permutations_windows(self): - project = create_project() - project.root_path = 'C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot' - - values = [ - "", - "server.js", - "~\\", - "~\\server.js", - "C:\\", - "C:\\server.js", - "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot", - "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\", - "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\server.js" - ] - values = ['C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot', 'server.js'] - - for file_name in values: - if file_name.endswith('server.js'): - for file_path in values: - out_file_path, absolute_path = project.get_full_file_path(file_path, file_name) - # print(f"file_path: {file_path} -> {out_file_path}, \tabsolute_path: {absolute_path}") - assert out_file_path == '', f'file_path: {file_path}, file_name: {file_name}' - # if absolute_path != '/Users/zvonimirsabljic/Development/copilot/pilot/server.js': - # print(f'file_path: {file_path}, file_name: {file_name}') - assert absolute_path == project.root_path + '/server.js',\ - f'file_path: {file_path}, file_name: {file_name}' - - def test_save_file_permutations_windows_deeper(self): - project = create_project() - project.root_path = 'C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot' - - values = [ - "", - "C:\\", - "~\\", - "fol der1\\fold er2\\server.js", - "C:\\folder1\\folder2\\server.js", - "~\\folder1\\folder2\\server.js", - "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\folder1\\folder2\\server.js", - - "folder1\\", - "C:\\folder1\\", - "~\\folder1\\", - "\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\folder1\\", - - "folder1", - "C:\\folder1", - "~\\folder1", - "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\folder1", - - "folder1\\folder2\\", - "C:\\folder1\\folder2\\", - "~\\folder1\\folder2\\", - "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\folder1\\folder2\\", - - "folder1\\folder2", - "C:\\folder1\\folder2", - "~\\folder1\\folder2", - "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\folder1\\folder2", - - "server.js", - "C:\\server.js", - "~\\server.js", - - "folder2\\server.js", - "C:\\folder2\\server.js", - "~\\folder2\\server.js", - "C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot\\folder2\\server.js", - ] - values = ['C:\\Users\\zvonimirsabljic\\Development\\copilot\\pilot', 'server.js'] - - for file_name in values: - if file_name.endswith('server.js'): - for file_path in values: - expected_path = '' - if 'folder1' in file_path: - if 'folder1/folder2' in file_path: - expected_path = 'folder1/folder2' - else: - expected_path = 'folder1' - elif 'folder2' in file_path: - expected_path = 'folder2' - - expected_absolute_path = project.root_path + \ - ('' if expected_path == '' else '/' + expected_path) + '/server.js' - - out_file_path, absolute_path = project.get_full_file_path(file_path, file_name) - # print(f"file_path: {file_path} -> {out_file_path}, \tabsolute_path: {absolute_path}") - assert out_file_path == expected_path, f'file_path: {file_path}, file_name: {file_name}' - # if absolute_path != expected_absolute_path: - # print(f'file_path: {file_path}, file_name: {file_name}') - assert absolute_path == expected_absolute_path, f'file_path: {file_path}, file_name: {file_name}' - - @pytest.mark.parametrize('test_data', [ - {'name': 'package.json', 'path': 'package.json', 'saved_to': '/temp/gpt-pilot-test/package.json'}, - {'name': 'package.json', 'path': '', 'saved_to': '/temp/gpt-pilot-test/package.json'}, - {'name': 'package.json', 'path': '/', 'saved_to': '/temp/gpt-pilot-test/package.json'}, # observed scenario - {'name': 'Dockerfile', 'path': None, 'saved_to': '/temp/gpt-pilot-test/Dockerfile'}, - {'name': None, 'path': 'public/index.html', 'saved_to': '/temp/gpt-pilot-test/public/index.html'}, - {'name': '', 'path': 'public/index.html', 'saved_to': '/temp/gpt-pilot-test/public/index.html'}, - - # TODO: Treatment of paths outside of the project workspace - https://github.com/Pythagora-io/gpt-pilot/issues/129 - # {'name': '/etc/hosts', 'path': None, 'saved_to': '/etc/hosts'}, - # {'name': '.gitconfig', 'path': '~', 'saved_to': '~/.gitconfig'}, - # {'name': '.gitconfig', 'path': '~/.gitconfig', 'saved_to': '~/.gitconfig'}, - # {'name': 'gpt-pilot.log', 'path': '/temp/gpt-pilot.log', 'saved_to': '/temp/gpt-pilot.log'}, - # ], ids=[ - # 'name == path', 'empty path', 'slash path', - # # 'None path', - # 'None name', 'empty name', - # 'None path absolute file', 'home path', 'home path same name', 'absolute path with name' - ]) - @patch('helpers.Project.update_file') - @patch('helpers.Project.File') - def test_save_file(self, mock_file_insert, mock_update_file, test_data): - # Given - data = {'content': 'Hello World!'} - if test_data['name'] is not None: - data['name'] = test_data['name'] - if test_data['path'] is not None: - data['path'] = test_data['path'] - - project = create_project() - - # When - project.save_file(data) - - # Then assert that update_file with the correct path - expected_saved_to = test_data['saved_to'] - mock_update_file.assert_called_once_with(expected_saved_to, 'Hello World!') - - # Also assert that File.insert was called with the expected arguments - # expected_file_data = {'app': project.app, 'path': test_data['path'], 'name': test_data['name'], - # 'full_path': expected_saved_to} - # mock_file_insert.assert_called_once_with(app=project.app, **expected_file_data, - # **{'name': test_data['name'], 'path': test_data['path'], - # 'full_path': expected_saved_to}) - - @pytest.mark.parametrize('file_path, file_name, expected', [ - ('file.txt', 'file.txt', '/temp/gpt-pilot-test/file.txt'), - ('', 'file.txt', '/temp/gpt-pilot-test/file.txt'), - ('path/', 'file.txt', '/temp/gpt-pilot-test/path/file.txt'), - ('path/to/', 'file.txt', '/temp/gpt-pilot-test/path/to/file.txt'), - ('path/to/file.txt', 'file.txt', '/temp/gpt-pilot-test/path/to/file.txt'), - ('./path/to/file.txt', 'file.txt', '/temp/gpt-pilot-test/path/to/file.txt'), - ]) - def test_get_full_path(self, file_path, file_name, expected): - # Given - project = create_project() - - # When - relative_path, absolute_path = project.get_full_file_path(file_path, file_name) - - # Then - assert absolute_path == expected - - @pytest.mark.skip(reason="Handling of absolute paths will be revisited in #29") - @pytest.mark.parametrize('file_path, file_name, expected', [ - ('/file.txt', 'file.txt', '/file.txt'), - ('/path/to/file.txt', 'file.txt', '/path/to/file.txt'), - # Only passes on Windows? ('C:\\path\\to\\file.txt', 'file.txt', 'C:\\path\\to/file.txt'), - ('~/path/to/file.txt', 'file.txt', '~/path/to/file.txt'), - ]) - def test_get_full_path_absolute(self, file_path, file_name, expected): - # Given - project = create_project() - - # When - relative_path, absolute_path = project.get_full_file_path(file_path, file_name) - - # Then - assert absolute_path == expected - - # # This is known to fail and should be avoided - # def test_get_full_file_path_error(self): - # # Given - # project = create_project() - # file_path = 'path/to/file/' - # file_name = '' - # - # # When - # relative_path, full_path = project.get_full_file_path(file_path, file_name) - # - # # Then - # assert full_path == '/temp/gpt-pilot-test/path/to/file/' +@pytest.mark.parametrize('test_data', [ + {'name': 'package.json', 'path': 'package.json', 'saved_to': '/temp/gpt-pilot-test/package.json'}, + {'name': 'package.json', 'path': '', 'saved_to': '/temp/gpt-pilot-test/package.json'}, + # {'name': 'Dockerfile', 'path': None, 'saved_to': '/temp/gpt-pilot-test/Dockerfile'}, + {'name': None, 'path': 'public/index.html', 'saved_to': '/temp/gpt-pilot-test/public/index.html'}, + {'name': '', 'path': 'public/index.html', 'saved_to': '/temp/gpt-pilot-test/public/index.html'}, + + # TODO: Treatment of paths outside of the project workspace - https://github.com/Pythagora-io/gpt-pilot/issues/129 + # {'name': '/etc/hosts', 'path': None, 'saved_to': '/etc/hosts'}, + # {'name': '.gitconfig', 'path': '~', 'saved_to': '~/.gitconfig'}, + # {'name': '.gitconfig', 'path': '~/.gitconfig', 'saved_to': '~/.gitconfig'}, + # {'name': 'gpt-pilot.log', 'path': '/temp/gpt-pilot.log', 'saved_to': '/temp/gpt-pilot.log'}, +], ids=[ + 'name == path', 'empty path', + # 'None path', + 'None name', 'empty name', + # 'None path absolute file', 'home path', 'home path same name', 'absolute path with name' +]) +@patch('helpers.Project.update_file') +@patch('helpers.Project.File') +def test_save_file(mock_file_insert, mock_update_file, test_data): + # Given + data = {'content': 'Hello World!'} + if test_data['name'] is not None: + data['name'] = test_data['name'] + if test_data['path'] is not None: + data['path'] = test_data['path'] + + project = create_project() + + # When + project.save_file(data) + + # Then assert that update_file with the correct path + expected_saved_to = test_data['saved_to'] + mock_update_file.assert_called_once_with(expected_saved_to, 'Hello World!') + + # Also assert that File.insert was called with the expected arguments + # expected_file_data = {'app': project.app, 'path': test_data['path'], 'name': test_data['name'], + # 'full_path': expected_saved_to} + # mock_file_insert.assert_called_once_with(app=project.app, **expected_file_data, + # **{'name': test_data['name'], 'path': test_data['path'], + # 'full_path': expected_saved_to}) + + +@pytest.mark.parametrize('file_path, file_name, expected', [ + ('file.txt', 'file.txt', '/temp/gpt-pilot-test/file.txt'), + ('', 'file.txt', '/temp/gpt-pilot-test/file.txt'), + ('path/', 'file.txt', '/temp/gpt-pilot-test/path/file.txt'), + ('path/to/', 'file.txt', '/temp/gpt-pilot-test/path/to/file.txt'), + ('path/to/file.txt', 'file.txt', '/temp/gpt-pilot-test/path/to/file.txt'), + ('./path/to/file.txt', 'file.txt', '/temp/gpt-pilot-test/./path/to/file.txt'), # ideally result would not have `./` +]) +def test_get_full_path(file_path, file_name, expected): + # Given + project = create_project() + + # When + relative_path, absolute_path = project.get_full_file_path(file_path, file_name) + + # Then + assert absolute_path == expected + + +@pytest.mark.skip(reason="Handling of absolute paths will be revisited in #29") +@pytest.mark.parametrize('file_path, file_name, expected', [ + ('/file.txt', 'file.txt', '/file.txt'), + ('/path/to/file.txt', 'file.txt', '/path/to/file.txt'), + # Only passes on Windows? ('C:\\path\\to\\file.txt', 'file.txt', 'C:\\path\\to/file.txt'), + ('~/path/to/file.txt', 'file.txt', '~/path/to/file.txt'), +]) +def test_get_full_path_absolute(file_path, file_name, expected): + # Given + project = create_project() + + # When + relative_path, absolute_path = project.get_full_file_path(file_path, file_name) + + # Then + assert absolute_path == expected + + +# This is known to fail and should be avoided +# def test_get_full_file_path_error(): +# # Given +# file_path = 'path/to/file/' +# file_name = '' +# +# # When +# full_path = project.get_full_file_path(file_path, file_name) +# +# # Then +# assert full_path == '/temp/gpt-pilot-test/path/to/file/' From 6bb54e3688bb9fa37cdc5c72dd0ba11505592cc1 Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Wed, 11 Oct 2023 16:30:25 +1100 Subject: [PATCH 19/21] fixed typo --- pilot/const/function_calls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pilot/const/function_calls.py b/pilot/const/function_calls.py index 7bfd6898a..a489b1034 100644 --- a/pilot/const/function_calls.py +++ b/pilot/const/function_calls.py @@ -140,7 +140,7 @@ def command_definition(description_command=f'A single command that needs to be e 'description': 'List of smaller development steps that need to be done to complete the entire task.', 'items': { 'type': 'object', - 'description': 'A smaller development step that needs to be done to complete the entire task. Remember, if you need to run a command that doesnt\'t finish by itself (eg. a command to run an app), put the timeout to 3000 milliseconds. If you need to create a directory that doesn\'t exist and is not the root project directory, always create it by running a command `mkdir`', + 'description': 'A smaller development step that needs to be done to complete the entire task. Remember, if you need to run a command that doesn\'t finish by itself (eg. a command to run an app), put the timeout to 3000 milliseconds. If you need to create a directory that doesn\'t exist and is not the root project directory, always create it by running a command `mkdir`', 'properties': { 'type': { 'type': 'string', From c7ad40a1e4a0c80522d4d6998553c69dabd3f9e6 Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Wed, 11 Oct 2023 18:21:46 +1100 Subject: [PATCH 20/21] include `kill_process` type --- pilot/const/function_calls.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pilot/const/function_calls.py b/pilot/const/function_calls.py index a489b1034..2619f5032 100644 --- a/pilot/const/function_calls.py +++ b/pilot/const/function_calls.py @@ -183,17 +183,17 @@ def command_definition(description_command=f'A single command that needs to be e 'description': 'List of smaller development steps that need to be done to complete the entire task.', 'items': { 'type': 'object', - 'description': 'A smaller development step that needs to be done to complete the entire task. Remember, if you need to run a command that doesnt\'t finish by itself (eg. a command to run an If you need to create a directory that doesn\'t exist and is not the root project directory, always create it by running a command `mkdir`', + 'description': 'A smaller development step that needs to be done to complete the entire task. Remember, if you need to run a command that doesn\'t finish by itself (eg. a command to run an If you need to create a directory that doesn\'t exist and is not the root project directory, always create it by running a command `mkdir`', 'properties': { 'type': { 'type': 'string', - 'enum': ['command', 'code_change', 'human_intervention'], + 'enum': ['command', 'kill_process', 'code_change', 'human_intervention'], 'description': 'Type of the development step that needs to be done to complete the entire task.', }, 'command': command_definition(), 'kill_process': { 'type': 'string', - 'description': 'To kill a process that was left running by a previous `command` provide the `process_name` in this field.' + 'description': 'To kill a process that was left running by a previous `command` step provide the `process_name` in this field and set `type` to "kill_process".', }, 'code_change': { 'type': 'object', From e4db222f6ee028034a67f4761d8e45676c400b6d Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Wed, 11 Oct 2023 18:23:23 +1100 Subject: [PATCH 21/21] test_terminate_process_not_running() --- pilot/helpers/test_cli.py | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 pilot/helpers/test_cli.py diff --git a/pilot/helpers/test_cli.py b/pilot/helpers/test_cli.py new file mode 100644 index 000000000..a39211988 --- /dev/null +++ b/pilot/helpers/test_cli.py @@ -0,0 +1,6 @@ +from pilot.helpers.cli import terminate_process + + +def test_terminate_process_not_running(): + terminate_process('999999999', 'not running') + assert True