feat: Add Uplink Terminal GUI and watcher script#54
Open
chromaglow wants to merge 4 commits intoverygoodplugins:mainfrom
Open
feat: Add Uplink Terminal GUI and watcher script#54chromaglow wants to merge 4 commits intoverygoodplugins:mainfrom
chromaglow wants to merge 4 commits intoverygoodplugins:mainfrom
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds an Automem Uplink Terminal: a README plus two new Python tools. A folder watcher ingests Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant GUI as HackerTerminal
participant FS as FileSystem
participant Watch as MemoryHandler
participant API as Automem API
User->>GUI: Click "INITIATE SEQUENCE"
GUI->>Watch: Start watchdog Observer
GUI->>FS: Ensure processed/ subdirectory exists
GUI->>GUI: Update status/UI
User->>FS: Drop file.txt into AutomemDropZone
FS->>Watch: on_created / on_moved event
Watch->>Watch: Read and prepare payload
Watch->>API: POST /memory (Bearer token)
alt 200/201 Success
API-->>Watch: Success
Watch->>FS: Move file to processed/ (rename on collision)
Watch->>GUI: Log success
GUI-->>User: Show success in console
else Failure
API-->>Watch: Error
Watch->>GUI: Log error
GUI-->>User: Show error in console
end
User->>GUI: Click "TERMINATE"
GUI->>Watch: Stop Observer and join
GUI->>GUI: Update status/UI to stopped
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@scripts/folder_watch.py`:
- Around line 75-92: The requests.post call that sends payload to
f"{BASE_URL}/memory" needs an explicit timeout to avoid hanging; add a timeout
parameter (e.g., timeout=10 or a constant like TIMEOUT_SECONDS) to the
requests.post invocation and update the exception handling to catch
requests.exceptions.Timeout (and/or requests.exceptions.RequestException) so
timeouts produce a clear error message instead of hanging; refer to the existing
resp = requests.post(f"{BASE_URL}/memory", json=payload, headers=HEADERS) line
and the except requests.exceptions.ConnectionError block to make these changes
and log a distinct message for timeouts.
- Around line 29-42: Remove the internal rambling comments and replace them with
a concise note that TOKEN is read from AUTOMEM_API_TOKEN and intentionally
defaults to empty (no hardcoded token). Keep BASE_URL and HEADERS as-is, but
change the startup behavior so when TOKEN is empty the module emits a clear
warning (use the module logger via logging.getLogger(__name__).warning or
similar) informing the user that no API token is set and that calls using
HEADERS will likely fail; do not default to any real token or silently proceed.
Ensure references to TOKEN and HEADERS remain intact so the warning reflects the
actual header construction.
- Around line 105-109: The on_moved method has inconsistent indentation (lines
within def on_moved use 3 spaces instead of 4) which will trigger
style/formatting errors; fix by normalizing the block to standard 4-space
indentation for the if, print, time.sleep, and process_file_util lines inside
def on_moved so the body aligns properly under def on_moved and maintains
consistent spacing around the event checks (event.is_directory,
event.dest_path.endswith, "processed" not in event.dest_path) and the call to
process_file_util(event.dest_path).
In `@scripts/watcher_gui.py`:
- Around line 124-140: The nested _check() function inside check_health uses
recursion (calling _check() after sleep) which will grow the call stack
indefinitely and also catches all exceptions with a bare except; replace the
recursive design with a while loop that runs while not self.root_destroyed,
performing the same requests.get to f"{BASE_URL}/health", updating GUI via
self.update_gui(lambda: self.brain_value.configure(...)) and sleeping 10s
between iterations, and change the bare except to catch specific exceptions
(e.g., requests.RequestException and optionally Exception for unexpected errors)
so you don't silence errors; keep launching the loop in a daemon thread as
before (threading.Thread(target=<loop_runner>, daemon=True).start()).
- Around line 19-35: The write method in class TextRedirector shadows the
built-in str and uses a bare except; rename the parameter from str to text
(update usages in TextRedirector.write) and replace the bare except with an
explicit except tk.TclError: pass (and ensure tkinter is imported as tk at top
if not already) so widget-destruction errors are handled without silencing other
problems.
In `@UPLINK_README.md`:
- Line 5: Replace the placeholder image Markdown element (the line containing
"")
with a real screenshot of the Uplink Terminal (hosted in the repo or a stable
CDN) or remove the image tag entirely; ensure the alt text remains descriptive
(e.g., "Uplink Terminal") and update the URL to point to the committed asset or
final production location.
🧹 Nitpick comments (7)
scripts/folder_watch.py (4)
1-10: Missing type hints throughout the module.As per coding guidelines, Python files must use type hints. The functions and methods in this module lack type annotations.
Example type hints for key functions
-def process_file_util(file_path): +def process_file_util(file_path: str) -> None:For the class methods:
-def on_created(self, event): +def on_created(self, event: FileSystemEvent) -> None: -def on_moved(self, event): +def on_moved(self, event: FileSystemEvent) -> None:
64-67: Shadowing module-levelHEADERSconstant inside function.The local
HEADERSvariable shadows the module-level constant defined at lines 39-42. This is redundant and confusing. Either use the module-level constant or rename the local variable.Proposed fix
- HEADERS = { - "Authorization": f"Bearer {TOKEN}", - "Content-Type": "application/json" - } - payload = { "content": content, "type": "Context", "metadata": {"source": "drop_zone", "filename": os.path.basename(file_path)} }Since
HEADERSis already defined at module level with the same values, remove the local redefinition.
94-95: Avoid catching bareException; catch specific exceptions.Catching
Exceptionbroadly can mask unexpected errors and make debugging difficult (BLE001). Consider catching specific exceptions likeIOError,OSError, orUnicodeDecodeErrorthat are likely during file processing.Proposed fix
- except Exception as e: - print(f"Error processing file: {e}") + except (IOError, OSError, UnicodeDecodeError) as e: + print(f"Error processing file: {e}")
117-122: Potential iteration over directory without checking for files vs. subdirectories.The startup scan at lines 119-122 iterates over
DROP_ZONEbut doesn't explicitly checkos.path.isfile(). While.endswith(".txt")likely filters out directories, it's defensive to add an explicit file check, especially sincePROCESSED_DIRis a subdirectory.Proposed fix
if os.path.exists(DROP_ZONE): for filename in os.listdir(DROP_ZONE): - if filename.endswith(".txt"): + if filename.endswith(".txt") and os.path.isfile(os.path.join(DROP_ZONE, filename)): filepath = os.path.join(DROP_ZONE, filename) process_file_util(filepath)scripts/watcher_gui.py (2)
182-188: Bareexceptinroot_destroyedproperty; prefer specific exception.The bare
except:at line 187 catches all exceptions. For checking widget existence, catchtk.TclErrorspecifically.Proposed fix
`@property` - def root_destroyed(self): + def root_destroyed(self) -> bool: try: - self.root.winfo_exists() - return False - except: + return not self.root.winfo_exists() + except tk.TclError: return True
37-113: Missing type hints onHackerTerminalclass methods.As per coding guidelines, type hints are required. Key methods like
__init__,animate,start_watcher,stop_watcher, etc. lack type annotations.Example type hints
def __init__(self, root: tk.Tk) -> None: def animate(self) -> None: def check_health(self) -> None: def update_gui(self, func: Callable[[], None]) -> None: def start_watcher(self) -> None: def stop_watcher(self) -> None:Add
from typing import Callableto imports.UPLINK_README.md (1)
16-21: Consider noting thattkintermay require separate installation on some Linux distributions.While the note mentions tkinter is included with standard Python, on Debian/Ubuntu-based systems it often requires
sudo apt install python3-tk. This could save users troubleshooting time.Suggested addition
pip install watchdog requests ``` - *(Note: `tkinter` is included with standard Python installations)* + *(Note: `tkinter` is included with standard Python installations on Windows/macOS. On Debian/Ubuntu, you may need: `sudo apt install python3-tk`)*
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@scripts/watcher_gui.py`:
- Around line 137-143: The lambda passed to update_gui closes over resp and may
see a different resp when executed later; capture resp.status_code eagerly by
binding it as a default argument in the lambda (e.g., lambda
status=resp.status_code: self.brain_value.configure(...)) so update_gui uses the
captured status, and do the same for the success branch (capture text/fg or
status) when calling self.update_gui and self.brain_value.configure.
- Around line 194-201: The root_destroyed property currently ignores the return
value of self.root.winfo_exists(); change it to read and use that result (call
exists = self.root.winfo_exists()) and return True when exists is 0 (or False
when exists is 1) instead of always returning False; update the property
(root_destroyed) to return the boolean equivalent of the winfo_exists() result
(e.g., return not bool(exists) or return exists == 0) while still catching
tk.TclError and returning True on exception.
- Around line 151-155: Add type hints across the module: import Callable from
typing, annotate the TextRedirector class methods (e.g., its __init__, write,
flush) and all public methods of HackerTerminal (including update_gui(self,
func), on_closing callback, and any other public methods) with appropriate
parameter and return types (e.g., func: Callable[[], None] or Callable[...,
None], self -> None for constructors, write -> None, flush -> None). Ensure
update_gui signature becomes update_gui(self, func: Callable[[], None]) -> None
and on_closing is typed (e.g., on_closing(self) -> None or on_closing(self,
event: Optional[Any]) -> None as appropriate), and add any necessary imports
(from typing import Callable, Optional, Any) to satisfy project standards.
- Around line 25-33: TextRedirector.write currently mutates the Tk widget
directly from background threads (e.g., triggered by MemoryHandler.on_created
prints), which is unsafe; change TextRedirector to enqueue incoming text into a
thread-safe queue.Queue and add a drain method scheduled on the GUI thread via
widget.after (or widget.after_idle) that dequeues and performs the
widget.configure/insert/see/update_idletasks operations; update any callers
(including print hooks or where TextRedirector is attached) to keep using
write() but rely on the internal queue so all actual widget access happens in
the main Tk thread.
chromaglow
force-pushed
the
feature/uplink-terminal
branch
from
f91eb1c to
d4ff405
Compare
Member
|
@codex - These errors in the pre-commit look like it's just maybe punctuation and spacing from Windows to Linux. Could you take a look at that and if that's the case, could you fix that for us here in the cloud? |
|
To use Codex here, create an environment for this repo. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds the Automem Uplink Terminal, a desktop integration tool that allows for automatic memory ingestion via a "Drop Zone" folder.
Features
~/Desktop/AutomemDropZone(configurable) and automatically ingests new .txt files into the Automem API.Motivation
Simplifies the ingestion workflow by allowing users to simply drag-and-drop text files to add memories, rather than using CLI commands or API calls manually.
Usage
python scripts/watcher_gui.pyAutomemDropZonefolder on the Desktop.