Address PR feedback

Author: AlePouroullis

src/humanloop/cli/__main__.py

@@ -26,8 +26,6 @@
 INFO_COLOR = "blue"
 WARNING_COLOR = "yellow"
 
-MAX_FILES_TO_DISPLAY = 10
-
 def get_client(api_key: Optional[str] = None, env_file: Optional[str] = None, base_url: Optional[str] = None) -> Humanloop:
     """Get a Humanloop client instance.
     
@@ -74,7 +72,7 @@ def common_options(f: Callable) -> Callable:
     )
     @click.option(
         "--base-dir",
-        help="Base directory for pulled files",
+        help="Base directory for pulled files (default: humanloop/)",
         default="humanloop",
         type=click.Path(),
     )
@@ -117,7 +115,7 @@ def cli(): # Does nothing because used as a group for other subcommands (pull, p
 @click.option(
     "--path",
     "-p",
-    help="Path in the Humanloop workspace to pull from (file or directory). You can pull an entire directory (e.g. 'my/directory') "
+    help="Path in the Humanloop workspace to pull from (file or directory). You can pull an entire directory (e.g. 'my/directory/') "
     "or a specific file (e.g. 'my/directory/my_prompt.prompt'). When pulling a directory, all files within that directory and its subdirectories will be included.",
     default=None,
 )
@@ -151,18 +149,18 @@ def pull(
     verbose: bool,
     quiet: bool
 ):
-    """Pull prompt and agent files from Humanloop to your local filesystem.
+    """Pull Prompt and Agent files from Humanloop to your local filesystem.
 
     \b
     This command will:
     1. Fetch Prompt and Agent files from your Humanloop workspace
-    2. Save them to your local filesystem
+    2. Save them to your local filesystem (default: humanloop/)
     3. Maintain the same directory structure as in Humanloop
     4. Add appropriate file extensions (.prompt or .agent)
 
     \b
     The files will be saved with the following structure:
-    {base_dir}/
+    humanloop/
     ├── my_project/
     │   ├── prompts/
     │   │   ├── my_prompt.prompt

src/humanloop/overload.py

@@ -176,13 +176,13 @@ def _overload(self, function_name: str, **kwargs) -> PromptCallResponse:
         # Handle local files if enabled
         if use_local_files and "path" in kwargs:
             # Check if version_id or environment is specified
-            has_version_info = "version_id" in kwargs or "environment" in kwargs
+            use_remote = any(["version_id" in kwargs, "environment" in kwargs])
             normalized_path = sync_client._normalize_path(kwargs["path"])
 
-            if has_version_info:
-                logger.warning(
-                    f"Ignoring local file for `{normalized_path}` as version_id or environment was specified. "
-                    "Using remote version instead."
+            if use_remote:
+                raise HumanloopRuntimeError(
+                    f"Cannot use local file for `{normalized_path}` as version_id or environment was specified. "
+                    "Please either remove version_id/environment to use local files, or set use_local_files=False to use remote files."
                 )
             else:
                 # Only use local file if no version info is specified

src/humanloop/sync/sync_client.py

@@ -6,6 +6,7 @@
 import time
 from humanloop.error import HumanloopRuntimeError
 import json
+import re
 
 if TYPE_CHECKING:
     from humanloop.base_client import BaseHumanloop
@@ -56,6 +57,9 @@ class SyncClient:
     manually cleared using the clear_cache() method.
     """
 
+    # File types that can be serialized to/from the filesystem
+    SERIALIZABLE_FILE_TYPES = ["prompt", "agent"]
+    
     def __init__(
         self, 
         client: "BaseHumanloop",
@@ -75,7 +79,6 @@ def __init__(
         self.base_dir = Path(base_dir)
         self._cache_size = cache_size
 
-        global logger 
         logger.setLevel(log_level)
 
         # Create a new cached version of get_file_content with the specified cache size
@@ -94,7 +97,10 @@ def _get_file_content_impl(self, path: str, file_type: FileType) -> str:
             The raw file content
             
         Raises:
-            HumanloopRuntimeError: If the file doesn't exist or can't be read
+            HumanloopRuntimeError: In two cases:
+                1. If the file doesn't exist at the expected location
+                2. If there's a filesystem error when trying to read the file
+                   (e.g., permission denied, file is locked, etc.)
         """
         # Construct path to local file
         local_path = self.base_dir / path
@@ -137,32 +143,17 @@ def clear_cache(self) -> None:
 
     def _normalize_path(self, path: str) -> str:
         """Normalize the path by:
-        1. Removing any file extensions (.prompt, .agent)
-        2. Converting backslashes to forward slashes
-        3. Removing leading and trailing slashes
-        4. Removing leading and trailing whitespace
-        5. Normalizing multiple consecutive slashes into a single forward slash
-
-        Args:
-            path: The path to normalize
-
-        Returns:
-            The normalized path
+        1. Converting to a Path object to handle platform-specific separators
+        2. Removing any file extensions
+        3. Converting to a string with forward slashes and no leading/trailing slashes
         """
-        # Remove any file extensions
-        path = path.rsplit('.', 1)[0] if '.' in path else path
-        
-        # Convert backslashes to forward slashes and normalize multiple slashes
-        path = path.replace('\\', '/')
+        # Convert to Path object to handle platform-specific separators
+        path_obj = Path(path)
 
-        # Remove leading/trailing whitespace and slashes
-        path = path.strip().strip('/')
-        
-        # Normalize multiple consecutive slashes into a single forward slash
-        while '//' in path:
-            path = path.replace('//', '/')
-            
-        return path
+        # Remove extension, convert to string with forward slashes, and remove leading/trailing slashes
+        normalized = str(path_obj.with_suffix(''))
+        # Replace all backslashes and normalize multiple forward slashes
+        return '/'.join(part for part in normalized.replace('\\', '/').split('/') if part)
 
     def is_file(self, path: str) -> bool:
         """Check if the path is a file by checking for .prompt or .agent extension."""
@@ -183,7 +174,7 @@ def _save_serialized_file(self, serialized_content: str, file_path: str, file_ty
             with open(new_path, "w") as f:
                 f.write(serialized_content)
 
-            # Clear the cache for this file to ensure we get fresh content next time
+            # Clear the cache when a file is saved
             self.clear_cache()
         except Exception as e:
             logger.error(f"Failed to sync {file_type} {file_path}: {str(e)}")
@@ -208,7 +199,7 @@ def _pull_file(self, path: str, environment: str | None = None) -> bool:
             logger.error(f"Failed to pull file {path}: {format_api_error(e)}")
             return False
 
-        if file.type not in ["prompt", "agent"]:
+        if file.type not in self.SERIALIZABLE_FILE_TYPES:
             raise ValueError(f"Unsupported file type: {file.type}")
 
         try:
@@ -240,7 +231,7 @@ def _pull_directory(self,
 
         while True:
             try:
-                logger.debug(f"Requesting page {page} of files")
+                logger.debug(f"`{path}`: Requesting page {page} of files")
                 response = self.client.files.list_files(
                     type=["prompt", "agent"], 
                     page=page,
@@ -250,15 +241,15 @@ def _pull_directory(self,
                 )
 
                 if len(response.records) == 0:
-                    logger.debug("No more files found")
+                    logger.debug(f"Finished reading files for path `{path}`")
                     break
 
-                logger.debug(f"Found {len(response.records)} files from page {page}")
+                logger.debug(f"`{path}`: Read page {page} containing {len(response.records)} files")
 
                 # Process each file
                 for file in response.records:
                     # Skip if not a Prompt or Agent
-                    if file.type not in ["prompt", "agent"]:
+                    if file.type not in self.SERIALIZABLE_FILE_TYPES:
                         logger.warning(f"Skipping unsupported file type: {file.type}")
                         continue
 
@@ -268,7 +259,7 @@ def _pull_directory(self,
                         continue
 
                     try:
-                        logger.debug(f"Saving {file.type} {file.path}")
+                        logger.debug(f"Writing {file.type} {file.path} to disk")
                         self._save_serialized_file(file.raw_file_content, file.path, file.type)
                         successful_files.append(file.path)
                     except Exception as e:
@@ -318,7 +309,7 @@ def pull(self, path: str | None = None, environment: str | None = None) -> Tuple
                 successful_files, failed_files = self._pull_directory(None, environment)
             else:
                 if self.is_file(path.strip()):
-                    logger.debug(f"Pulling specific file: {normalized_path}")
+                    logger.debug(f"Pulling file: {normalized_path}")
                     if self._pull_file(normalized_path, environment):
                         successful_files = [path]
                         failed_files = []