From 14671760f730c683fbb215686add8524d2cef88d Mon Sep 17 00:00:00 2001 From: Ben Morehouse Date: Fri, 4 Apr 2025 17:42:22 -0700 Subject: [PATCH] ben suggestions Signed-off-by: Ben Morehouse --- Makefile | 0 feedback.md | 23 ++++++++ main.py | 1 + .../toast/command_executor.py | 0 .../toast/command_generator.py | 8 ++- file_manager.py => src/toast/file_manager.py | 0 toast => src/toast/toast.py | 54 +++++++++---------- 7 files changed, 55 insertions(+), 31 deletions(-) create mode 100644 Makefile create mode 100644 feedback.md create mode 100644 main.py rename command_executor.py => src/toast/command_executor.py (100%) rename command_generator.py => src/toast/command_generator.py (95%) rename file_manager.py => src/toast/file_manager.py (100%) rename toast => src/toast/toast.py (95%) diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..e69de29 diff --git a/feedback.md b/feedback.md new file mode 100644 index 0000000..749f093 --- /dev/null +++ b/feedback.md @@ -0,0 +1,23 @@ +# Feedback + +- you don't want people to have to run `pip install`. Instead you can ship a binary in github and have people just pull the binary +Create a release here: https://github.com/billybjork/pixel-toaster/releases + +- create a proper entry point to your application + +- organize your cdoe more where the main.py just shows the different modes that your cli can run, and then imports code from your src directory. + +- When someone pulls your toast binary, have it on the first run no mattter what go in and run commands to get people's input, such as + - what device are you on + - what is your open api key + And then you save it to ~/.toast.conf or ~/.toastrc + +- familiarize yourself with dotfiles + +- When someone comes to you with problems, you will have nothing to be able to debug. Your initialization should save a `~/.toast.logs` as well as a `~/.toast.history` +- now that I am thinking about it, there should just be a directory created called `~/.toast_bjork` directory + +- pass in a command that says " i want to check this ffmpeg command first before running it " +- use `makefile` stuff to create a binary of your project, and then a separate command to push and publish it to your github repo. +- in your makefile also add some stuff in for linters, this will make your code cleaner and give you automatic feedback + https://codilime.com/blog/python-code-quality-linters/ diff --git a/main.py b/main.py new file mode 100644 index 0000000..e0c3920 --- /dev/null +++ b/main.py @@ -0,0 +1 @@ +# This file should only show how this thing runs diff --git a/command_executor.py b/src/toast/command_executor.py similarity index 100% rename from command_executor.py rename to src/toast/command_executor.py diff --git a/command_generator.py b/src/toast/command_generator.py similarity index 95% rename from command_generator.py rename to src/toast/command_generator.py index b4403c4..9b1d800 100644 --- a/command_generator.py +++ b/src/toast/command_generator.py @@ -5,6 +5,7 @@ import openai from typing import List, Dict, Optional +# TODO Put this command into its own text file class CommandGenerator: # --- Enhanced System Prompt --- SYSTEM_PROMPT_TEMPLATE = """\ @@ -84,10 +85,12 @@ def clean_json_response(self, response_str: str) -> str: return response_str + # TODO: no need to use `List`, you can just use list, same with dict def generate_command(self, conversation_history: List[Dict[str, str]], system_context: Dict[str, str]) -> str: """ Generates the FFmpeg command using the LLM, incorporating context and conversation history. """ + # TODO try to break down this funtion a bit more # Format the file context message for the system prompt file_context_str = "\nFILE CONTEXT:\n" explicit_file = system_context.get("explicit_input_file") @@ -108,6 +111,7 @@ def generate_command(self, conversation_history: List[Dict[str, str]], system_co else: relative_files_for_prompt.append(f_abs) except ValueError: # Files on different drives (Windows) + # TODO: are you sure you want to do this here vs fail? relative_files_for_prompt.append(f_abs) files_list_str = ", ".join([f"'{f}'" for f in relative_files_for_prompt]) @@ -165,9 +169,11 @@ def generate_command(self, conversation_history: List[Dict[str, str]], system_co return content + # TODO: Put this not here but in the root of toast.py, have something that catches all error types. Makes this + # part of your code much cleaner, and common of repos to put all error handling into general place for python except openai.APIConnectionError as e: logging.error(f"OpenAI API connection error: {e}"); raise except openai.RateLimitError as e: logging.error(f"OpenAI API rate limit exceeded: {e}"); raise except openai.AuthenticationError as e: logging.error(f"OpenAI API authentication error (invalid key?): {e}"); raise except openai.PermissionDeniedError as e: logging.error(f"OpenAI API permission denied error: {e}"); raise except openai.APIStatusError as e: logging.error(f"OpenAI API status error: {e.status_code} - {e.response}"); raise - except Exception as e: logging.error(f"Error during OpenAI API call: {e}", exc_info=True); raise \ No newline at end of file + except Exception as e: logging.error(f"Error during OpenAI API call: {e}", exc_info=True); raise diff --git a/file_manager.py b/src/toast/file_manager.py similarity index 100% rename from file_manager.py rename to src/toast/file_manager.py diff --git a/toast b/src/toast/toast.py similarity index 95% rename from toast rename to src/toast/toast.py index 571ac8d..a6a3532 100755 --- a/toast +++ b/src/toast/toast.py @@ -10,6 +10,15 @@ from pathlib import Path from typing import Tuple, Optional from dotenv import load_dotenv +from file_manager import ( + FileManager, + VIDEO_EXTENSIONS, + IMAGE_EXTENSIONS, + AUDIO_EXTENSIONS +) +from command_generator import CommandGenerator +from command_executor import CommandExecutor +import openai # --- Determine paths --- # Get the path of the script file, resolving symlinks @@ -20,6 +29,7 @@ # __file__ might not be defined if running interactively or packaged weirdly REAL_SCRIPT_FILE = os.path.abspath(sys.argv[0]) # Fallback using argv +# TODO: a lot of this is going to instead come from your ~/.toast_bjork SCRIPT_DIR = os.path.dirname(REAL_SCRIPT_FILE) # Directory of the real script file CURRENT_WORKDIR = os.getcwd() USER_HOME = Path.home() @@ -47,35 +57,7 @@ sys.path.insert(0, SCRIPT_DIR) logging.debug(f"Inserted script directory into sys.path: {SCRIPT_DIR}") -try: - # *** Import constants directly from file_manager *** - from file_manager import ( - FileManager, - VIDEO_EXTENSIONS, - IMAGE_EXTENSIONS, - AUDIO_EXTENSIONS - ) - from command_generator import CommandGenerator - from command_executor import CommandExecutor - logging.debug("Helper modules and constants imported successfully.") -except ImportError as e: - # Log error using the configured logger - logging.error(f"Failed to import required modules/constants.") - logging.error(f" Real Script Directory: {SCRIPT_DIR}") - logging.error(f" Current sys.path: {sys.path}") - logging.error(f" Ensure file_manager.py (with constants defined), command_generator.py, command_executor.py") - logging.error(f" are in the script directory or installed correctly.") - logging.exception("Import Error details:") # Logs the full traceback - sys.exit(1) - -# --- Import openai here, AFTER potential error exit --- -# The actual configuration happens later after key check -try: - import openai -except ImportError: - logging.error("'openai' library not installed. Please install it using 'pip install openai'.", exc_info=True) - sys.exit(1) - +# TODO: move this to `utils.py` def print_art(): art = r""" ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢀⣄⠀⠀⠀⢀⡀⠀⠀⠀⣀⡀⠀⠀⠀⠀⠀⠀ @@ -96,16 +78,22 @@ def print_art(): """ print(art) +# TODO: move this to its own loader class + # --- System Info Gathering --- def get_ffmpeg_executable() -> str: ffmpeg_path = shutil.which("ffmpeg") if ffmpeg_path is None: raise FileNotFoundError("Missing ffmpeg executable.") return ffmpeg_path +# TODO: let these functions breath, eg put space between beginning and end. +# TODO: use a linter if you don't already, it should catch this def get_ffmpeg_version(ffmpeg_exe: str) -> str: try: result = subprocess.run([ffmpeg_exe, "-version"], capture_output=True, text=True, check=False, timeout=5); output = result.stdout if "ffmpeg version" in result.stdout.lower() else result.stderr; first_line = output.splitlines()[0].strip() if output and output.splitlines() else "Could not determine version"; return first_line if "version" in first_line.lower() else output except Exception as e: logging.warning(f"Could not get ffmpeg version: {e}"); return "Unknown" def get_os_info() -> Tuple[str, str]: os_type = platform.system(); return os_type, f"{os_type} {platform.release()} {platform.machine()}" + +# TODO: cleaner way to do this def get_default_shell() -> Optional[str]: shell = os.getenv("SHELL") if shell: return shell @@ -117,6 +105,7 @@ def get_default_shell() -> Optional[str]: return None # --- Main Execution Logic --- +# TODO: main.py instead of here, and keep the logic in it as light as possible def main(): parser = argparse.ArgumentParser( description="toast: Natural language FFmpeg command generator." @@ -152,6 +141,10 @@ def main(): logging.error(f"Initialization failed: {e}") sys.exit(1) except Exception as e: + # TODO: update all your logging like this, also make an alias from logging to log + # so you can just do log.warning, insterad of logging + # logging.warning(",... %s ", args.verbose) + # logging.warning("Could not gather some system info: %s", args.verbose) # Show traceback if verbose logging.warning(f"Could not gather some system info: {e}", exc_info=args.verbose) # Show traceback if verbose ffmpeg_executable = "ffmpeg"; ffmpeg_version = "Unknown" os_type, os_info = "Unknown", "Unknown"; default_shell = "Unknown" @@ -408,6 +401,7 @@ def main(): # --- Final Check for API Key --- if not api_key: # Print error to stderr for visibility even if logging isn't working fully + # TODO: use a differnet more structured logger eprint = lambda *a, **k: print(*a, file=sys.stderr, **k) eprint(f"\n[ERROR] OPENAI_API_KEY could not be found.") eprint(f" Search Order:") @@ -448,4 +442,4 @@ def main(): sys.exit(130) # Standard exit code for Ctrl+C except Exception as e: logging.exception("An unhandled exception occurred outside the main loop:") - sys.exit(1) # General error exit code \ No newline at end of file + sys.exit(1) # General error exit code