Skip to content

Commit 9b7c809

Browse files
committed
Further refined path handling and added more tests
1 parent 32f0b1b commit 9b7c809

File tree

7 files changed

+125
-111
lines changed

7 files changed

+125
-111
lines changed

src/humanloop/overload.py

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,30 @@
11
import inspect
22
import logging
33
import types
4-
from typing import Any, Dict, Optional, Union, Callable
4+
from pathlib import Path
5+
from typing import Any, Callable, Dict, Optional, Union
56

7+
from humanloop import path_utils
8+
from humanloop.agents.client import AgentsClient
69
from humanloop.context import (
710
get_decorator_context,
811
get_evaluation_context,
912
get_trace_id,
1013
)
14+
from humanloop.datasets.client import DatasetsClient
1115
from humanloop.error import HumanloopRuntimeError
12-
from humanloop.sync.sync_client import SyncClient
13-
from humanloop.prompts.client import PromptsClient
16+
from humanloop.evaluators.client import EvaluatorsClient
1417
from humanloop.flows.client import FlowsClient
15-
from humanloop.datasets.client import DatasetsClient
16-
from humanloop.agents.client import AgentsClient
18+
from humanloop.prompts.client import PromptsClient
19+
from humanloop.sync.sync_client import SyncClient
1720
from humanloop.tools.client import ToolsClient
18-
from humanloop.evaluators.client import EvaluatorsClient
1921
from humanloop.types import FileType
22+
from humanloop.types.agent_call_response import AgentCallResponse
2023
from humanloop.types.create_evaluator_log_response import CreateEvaluatorLogResponse
2124
from humanloop.types.create_flow_log_response import CreateFlowLogResponse
2225
from humanloop.types.create_prompt_log_response import CreatePromptLogResponse
2326
from humanloop.types.create_tool_log_response import CreateToolLogResponse
2427
from humanloop.types.prompt_call_response import PromptCallResponse
25-
from humanloop.types.agent_call_response import AgentCallResponse
2628

2729
logger = logging.getLogger("humanloop.sdk")
2830

@@ -95,16 +97,21 @@ def _handle_local_files(
9597

9698
path = kwargs["path"]
9799

98-
# Check if the path has a file type extension (.prompt/.agent)
99-
if sync_client.is_file(path):
100-
file_type = _get_file_type_from_client(client)
100+
# First check for path format issues (absolute paths or leading/trailing slashes)
101+
normalized_path = path.strip("/")
102+
if Path(path).is_absolute() or path != normalized_path:
103+
raise HumanloopRuntimeError(
104+
f"Path '{path}' format is invalid. "
105+
f"Paths must follow the standard API format 'path/to/resource' without leading or trailing slashes. "
106+
f"Please use '{normalized_path}' instead."
107+
)
101108

102-
# Safely extract the extension and path by handling potential errors
109+
# Then check for file extensions
110+
if sync_client.is_file(path):
103111
try:
104112
parts = path.rsplit(".", 1)
105113
path_without_extension = parts[0] if len(parts) > 0 else path
106114
except Exception:
107-
# Fallback to original path if any error occurs
108115
path_without_extension = path
109116

110117
raise HumanloopRuntimeError(
@@ -124,7 +131,7 @@ def _handle_local_files(
124131

125132
file_type = _get_file_type_from_client(client)
126133
if file_type not in SyncClient.SERIALIZABLE_FILE_TYPES:
127-
raise HumanloopRuntimeError(f"Local files are not supported for `{file_type}` files.")
134+
raise HumanloopRuntimeError(f"Local files are not supported for `{file_type.capitalize()}` files: '{path}'.")
128135

129136
# If file_type is already specified in kwargs (prompt or agent), it means user provided a Prompt- or AgentKernelRequestParams object
130137
if file_type in kwargs and not isinstance(kwargs[file_type], str):

src/humanloop/path_utils.py

Lines changed: 15 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,22 @@
11
from pathlib import Path
2-
from typing import Optional
32

43
from humanloop.error import HumanloopRuntimeError
54

65

7-
def validate_no_extensions(path: str, allowed_extensions: Optional[list[str]] = None) -> str:
8-
"""Validates that path has no extensions (or only allowed ones) and returns path without extension."""
9-
if "." in path:
10-
parts = path.rsplit(".", 1)
11-
extension = parts[1] if len(parts) > 1 else ""
12-
path_without_extension = parts[0]
13-
14-
if allowed_extensions and extension in allowed_extensions:
15-
return path
16-
17-
raise HumanloopRuntimeError(
18-
f"Path '{path}' includes a file extension which is not supported in this context. "
19-
f"Use the format without extensions: '{path_without_extension}'."
20-
)
21-
return path
6+
def normalize_path(path: str, strip_extension: bool = False) -> str:
7+
"""Normalize a path to the standard API format: path/to/resource, where resource can be a file or a directory."""
8+
# Remove leading/trailing slashes
9+
path_obj = Path(path.strip("/"))
2210

23-
def validate_no_slashes(path: str) -> str:
24-
"""Validates that path has no leading/trailing slashes."""
25-
if path != path.strip("/"):
26-
raise HumanloopRuntimeError(
27-
f"Invalid path: '{path}'. Path should not contain leading/trailing slashes. "
28-
f"Valid example: 'path/to/resource'"
29-
)
30-
return path
11+
# Check if path is absolute
12+
if path_obj.is_absolute():
13+
raise HumanloopRuntimeError("Absolute paths are not supported. Ensure the ")
3114

32-
def validate_not_absolute(path: str, base_dir: Optional[str] = None) -> str:
33-
"""Validates that path is not absolute."""
34-
if Path(path).is_absolute():
35-
message = f"Absolute paths are not supported: '{path}'."
36-
if base_dir:
37-
message += f" Paths should be relative to '{base_dir}'."
38-
raise HumanloopRuntimeError(message)
39-
return path
15+
# Handle extension
16+
if strip_extension:
17+
normalized = str(path_obj.with_suffix(""))
18+
else:
19+
normalized = str(path_obj)
20+
21+
# Normalize separators
22+
return "/".join(part for part in normalized.replace("\\", "/").split("/") if part)

src/humanloop/sync/sync_client.py

Lines changed: 8 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1+
import json
12
import logging
2-
from pathlib import Path
3-
from typing import List, Optional, Tuple, TYPE_CHECKING, Union
4-
from functools import lru_cache
5-
import typing
63
import time
4+
import typing
5+
from functools import lru_cache
6+
from pathlib import Path
7+
from typing import TYPE_CHECKING, List, Optional, Tuple
8+
9+
from humanloop import path_utils
710
from humanloop.error import HumanloopRuntimeError
8-
import json
911

1012
if TYPE_CHECKING:
1113
from humanloop.base_client import BaseHumanloop
@@ -154,30 +156,6 @@ def clear_cache(self) -> None:
154156
"""Clear the LRU cache."""
155157
self.get_file_content.cache_clear() # type: ignore [attr-defined]
156158

157-
def _normalize_path(self, path: str, strip_extension: bool = False) -> str:
158-
"""Normalize the path by:
159-
1. Converting to a Path object to handle platform-specific separators
160-
2. Removing any file extensions if strip_extension is True
161-
3. Converting to a string with forward slashes and no leading/trailing slashes
162-
"""
163-
# Convert to Path object to handle platform-specific separators
164-
path_obj = Path(path)
165-
166-
# Reject absolute paths to ensure all paths are relative to base_dir.
167-
# This maintains consistency with the remote filesystem where paths are relative to project root.
168-
if path_obj.is_absolute():
169-
raise HumanloopRuntimeError(
170-
f"Absolute paths are not supported: `{path}`. "
171-
f"Paths should be relative to the base directory (`{self.base_dir}`)."
172-
)
173-
174-
# Remove extension, convert to string with forward slashes, and remove leading/trailing slashes
175-
if strip_extension:
176-
normalized = str(path_obj.with_suffix(""))
177-
else:
178-
normalized = str(path_obj)
179-
# Replace all backslashes and normalize multiple forward slashes
180-
return "/".join(part for part in normalized.replace("\\", "/").split("/") if part)
181159

182160
def is_file(self, path: str) -> bool:
183161
"""Check if the path is a file by checking for .{file_type} extension for serializable file types.
@@ -369,7 +347,7 @@ def pull(self, path: Optional[str] = None, environment: Optional[str] = None) ->
369347
is_file_path = self.is_file(path)
370348

371349
# For API communication, we need path without extension
372-
api_path = self._normalize_path(path, strip_extension=True)
350+
api_path = path_utils.normalize_path(path, strip_extension=True)
373351

374352
logger.info(f"Starting pull: path={api_path or '(root)'}, environment={environment or '(default)'}")
375353

tests/custom/integration/test_overload.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,9 @@ def test_path_validation(
5454
f"//{test_file.path}//", # Multiple leading and trailing slashes
5555
]
5656

57-
# THEN appropriate error should be raised
57+
# THEN appropriate error should be raised about slashes
5858
for path in slash_paths:
59-
with pytest.raises(HumanloopRuntimeError, match=""):
59+
with pytest.raises(HumanloopRuntimeError, match="Path .* format is invalid"):
6060
if test_file.type == "prompt":
6161
humanloop_client.prompts.call(path=path, messages=[{"role": "user", "content": "Testing"}])
6262
elif test_file.type == "agent":
@@ -68,9 +68,9 @@ def test_path_validation(
6868
f"/{test_file.path}.{test_file.type}", # Extension and leading slash
6969
]
7070

71-
# THEN the extension error should be prioritized (more specific validation first)
71+
# THEN the format validation error should be raised first (before extension validation)
7272
for path in combined_paths:
73-
with pytest.raises(HumanloopRuntimeError, match="includes a file extension which is not supported"):
73+
with pytest.raises(HumanloopRuntimeError, match="Path .* format is invalid"):
7474
if test_file.type == "prompt":
7575
humanloop_client.prompts.call(path=path, messages=[{"role": "user", "content": "Testing"}])
7676
elif test_file.type == "agent":

tests/custom/integration/test_sync.py

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,58 @@ def test_pull_with_invalid_path(
4444
with pytest.raises(HumanloopRuntimeError, match=f"Directory `{non_existent_path}` does not exist"):
4545
humanloop_client.pull(path=non_existent_path)
4646

47+
4748
def test_pull_with_invalid_environment(
4849
get_humanloop_client: GetHumanloopClientFn,
4950
):
5051
"""Test that humanloop.sync() raises an error when the environment is invalid"""
5152
humanloop_client = get_humanloop_client()
52-
with pytest.raises(HumanloopRuntimeError, match="Invalid environment"):
53-
humanloop_client.pull(environment="invalid_environment")
53+
with pytest.raises(HumanloopRuntimeError, match="Environment .* does not exist"):
54+
humanloop_client.pull(environment="invalid_environment")
55+
56+
57+
# def test_pull_with_environment(
58+
# get_humanloop_client: GetHumanloopClientFn,
59+
# syncable_files_fixture: list[SyncableFile],
60+
# ):
61+
# """Test that humanloop.sync() correctly syncs files from a specific environment"""
62+
# # NOTE: This test is currently not feasible to implement because:
63+
# # 1. We have no way of deploying to an environment using its name, only by ID
64+
# # 2. There's no API endpoint to retrieve environments for an organization
65+
# #
66+
# # If implemented, this test would:
67+
# # 1. Deploy one of the syncable files to a specific environment (e.g., "production" as it's non-default)
68+
# # 2. Pull files filtering by the production environment
69+
# # 3. Check if the deployed file is present in the local filesystem
70+
# # 4. Verify that none of the other syncable files (that weren't deployed to production) are present
71+
# # This would confirm that environment filtering works correctly
72+
73+
74+
def test_pull_with_path_filter(
75+
get_humanloop_client: GetHumanloopClientFn,
76+
syncable_files_fixture: list[SyncableFile],
77+
sdk_test_dir: str,
78+
):
79+
"""Test that humanloop.sync() correctly filters files by path when pulling"""
80+
# GIVEN a client
81+
humanloop_client = get_humanloop_client()
82+
83+
# First clear any existing files to ensure clean state
84+
import shutil
85+
86+
if Path("humanloop").exists():
87+
shutil.rmtree("humanloop")
88+
89+
# WHEN pulling only files from the sdk_test_dir path
90+
humanloop_client.pull(path=sdk_test_dir)
91+
92+
# THEN count the total number of files synced
93+
synced_file_count = 0
94+
for path in Path("humanloop").glob("**/*"):
95+
if path.is_file():
96+
synced_file_count += 1
97+
98+
# The count should match our fixture length
99+
assert synced_file_count == len(syncable_files_fixture), (
100+
f"Expected {len(syncable_files_fixture)} files, got {synced_file_count}"
101+
)

tests/custom/sync/test_client.py

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -33,36 +33,6 @@ def test_init(sync_client: SyncClient, tmp_path: Path):
3333
assert sync_client.SERIALIZABLE_FILE_TYPES == frozenset(["prompt", "agent"])
3434

3535

36-
def test_normalize_path(sync_client: SyncClient):
37-
"""Test path normalization functionality."""
38-
# GIVEN various file paths with different formats
39-
test_cases = [
40-
# Input path, expected with strip_extension=False, expected with strip_extension=True
41-
("path/to/file.prompt", "path/to/file.prompt", "path/to/file"),
42-
("path\\to\\file.agent", "path/to/file.agent", "path/to/file"),
43-
("trailing/slashes/file.agent/", "trailing/slashes/file.agent", "trailing/slashes/file"),
44-
("multiple//slashes//file.prompt", "multiple/slashes/file.prompt", "multiple/slashes/file"),
45-
]
46-
47-
# Test with strip_extension=False (default)
48-
for input_path, expected_with_ext, _ in test_cases:
49-
# WHEN they are normalized without stripping extension
50-
normalized = sync_client._normalize_path(input_path, strip_extension=False)
51-
# THEN they should be converted to the expected format with extension
52-
assert normalized == expected_with_ext
53-
54-
# Test with strip_extension=True
55-
for input_path, _, expected_without_ext in test_cases:
56-
# WHEN they are normalized with extension stripping
57-
normalized = sync_client._normalize_path(input_path, strip_extension=True)
58-
# THEN they should be converted to the expected format without extension
59-
assert normalized == expected_without_ext
60-
61-
# Test absolute path raises error
62-
with pytest.raises(HumanloopRuntimeError, match="Absolute paths are not supported"):
63-
sync_client._normalize_path("/leading/slashes/file.prompt")
64-
65-
6636
def test_is_file(sync_client: SyncClient):
6737
"""Test file type detection with case insensitivity."""
6838
# GIVEN a SyncClient instance
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
from humanloop import path_utils
2+
3+
4+
def test_normalize_path():
5+
"""Test path normalization functionality."""
6+
# GIVEN various file paths with different formats
7+
test_cases = [
8+
# Input path, expected with strip_extension=False, expected with strip_extension=True
9+
("path/to/file.prompt", "path/to/file.prompt", "path/to/file"),
10+
("path\\to\\file.agent", "path/to/file.agent", "path/to/file"),
11+
("/leading/slashes/file.prompt", "leading/slashes/file.prompt", "leading/slashes/file"),
12+
("trailing/slashes/file.agent/", "trailing/slashes/file.agent", "trailing/slashes/file"),
13+
("multiple//slashes//file.prompt", "multiple/slashes/file.prompt", "multiple/slashes/file"),
14+
]
15+
16+
# Test with strip_extension=False (default)
17+
for input_path, expected_with_ext, _ in test_cases:
18+
# WHEN they are normalized without stripping extension
19+
normalized = path_utils.normalize_path(input_path, strip_extension=False)
20+
# THEN they should be converted to the expected format with extension
21+
assert normalized == expected_with_ext
22+
23+
# Test with strip_extension=True
24+
for input_path, _, expected_without_ext in test_cases:
25+
# WHEN they are normalized with extension stripping
26+
normalized = path_utils.normalize_path(input_path, strip_extension=True)
27+
# THEN they should be converted to the expected format without extension
28+
assert normalized == expected_without_ext

0 commit comments

Comments
 (0)