-
Notifications
You must be signed in to change notification settings - Fork 1k
fix(traceloop-sdk): add type-checking support with mypy #3463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
a0f3129
c6cb82f
5ea4dd6
ba4e094
bae182e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,36 +8,35 @@ class UserFeedback(BaseAnnotation): | |
| def __init__(self, http: HTTPClient, app_name: str): | ||
| super().__init__(http, app_name, "user_feedback") | ||
|
|
||
|
|
||
| def create( | ||
| self, | ||
| annotation_task: str, | ||
| entity_instance_id: str, | ||
| tags: Dict[str, Any], | ||
| ) -> None: | ||
| """Create an annotation for a specific task. | ||
|
|
||
| Args: | ||
| annotation_task (str): The ID/slug of the annotation task to report to. | ||
| Can be found at app.traceloop.com/annotation_tasks/:annotation_task_id | ||
| entity_instance_id (str): The ID of the specific entity instance being annotated, should be reported | ||
| in the association properties | ||
| tags (Dict[str, Any]): Dictionary containing the tags to be reported. | ||
| Should match the tags defined in the annotation task | ||
|
|
||
| Example: | ||
| ```python | ||
| client = Client(api_key="your-key") | ||
| client.annotation.create( | ||
| annotation_task="task_123", | ||
| entity_instance_id="instance_456", | ||
| tags={ | ||
| "sentiment": "positive", | ||
| "relevance": 0.95, | ||
| "tones": ["happy", "nice"] | ||
| }, | ||
| ) | ||
| ``` | ||
| """ | ||
|
|
||
| return BaseAnnotation.create(self, annotation_task, entity_instance_id, tags) | ||
| def create( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like an un needed change |
||
| self, | ||
| annotation_task: str, | ||
| entity_instance_id: str, | ||
| tags: Dict[str, Any], | ||
| ) -> None: | ||
| """Create an annotation for a specific task. | ||
|
|
||
| Args: | ||
| annotation_task (str): The ID/slug of the annotation task to report to. | ||
| Can be found at app.traceloop.com/annotation_tasks/:annotation_task_id | ||
| entity_instance_id (str): The ID of the specific entity instance being annotated, should be reported | ||
| in the association properties | ||
| tags (Dict[str, Any]): Dictionary containing the tags to be reported. | ||
| Should match the tags defined in the annotation task | ||
|
|
||
| Example: | ||
| ```python | ||
| client = Client(api_key="your-key") | ||
| client.annotation.create( | ||
| annotation_task="task_123", | ||
| entity_instance_id="instance_456", | ||
| tags={ | ||
| "sentiment": "positive", | ||
| "relevance": 0.95, | ||
| "tones": ["happy", "nice"] | ||
| }, | ||
| ) | ||
| ``` | ||
| """ | ||
|
|
||
| return BaseAnnotation.create(self, annotation_task, entity_instance_id, tags) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,4 +63,5 @@ def __init__( | |
| self.user_feedback = UserFeedback(self._http, self.app_name) | ||
| self.datasets = Datasets(self._http) | ||
| experiment_slug = os.getenv("TRACELOOP_EXP_SLUG") | ||
| self.experiment = Experiment(self._http, self._async_http, experiment_slug) | ||
| # TODO: Fix type - Experiment constructor should accept Optional[str] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ? |
||
| self.experiment = Experiment(self._http, self._async_http, experiment_slug) # type: ignore[arg-type] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| from typing import Optional, TypeVar, Callable, Any, ParamSpec, Awaitable | ||
| from typing import Optional, TypeVar, Callable | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| import warnings | ||
|
|
||
| from opentelemetry.semconv_ai import TraceloopSpanKindValues | ||
|
|
@@ -8,9 +8,7 @@ | |
| entity_method, | ||
| ) | ||
|
|
||
| P = ParamSpec("P") | ||
| R = TypeVar("R") | ||
| F = TypeVar("F", bound=Callable[P, R | Awaitable[R]]) | ||
|
Comment on lines
-11
to
-13
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I got this with the agent after allowing mypy The issue: The TypeVar F has a bound that uses ParamSpec and is too strict:
Why It Looks Good in traceloop-sdkThe decorator types look correct syntactically, but they're overly restrictive for real-world usage patterns. The traceloop-sdk itself doesn't strictly type-check the decorators folder, so this issue wasn't caught in their own type checking. |
||
| F = TypeVar("F") | ||
|
|
||
|
|
||
| def task( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,13 +52,13 @@ async def wait_for_result( | |
| except Exception as e: | ||
| raise Exception(f"Unexpected error in SSE stream: {e}") | ||
|
|
||
| async def _handle_sse_response(self, response) -> ExecutionResponse: | ||
| async def _handle_sse_response(self, response: httpx.Response) -> ExecutionResponse: | ||
| """Handle SSE response: check status and parse result""" | ||
| if response.status_code != 200: | ||
| error_text = await response.aread() | ||
| raise Exception( | ||
| f"Failed to stream results: {response.status_code}, body: {error_text}" | ||
| ) | ||
| # TODO: Fix bytes formatting - should decode error_text or use !r | ||
| error_msg = f"Failed to stream results: {response.status_code}, body: {error_text}" # type: ignore[str-bytes-safe] # noqa: E501 | ||
| raise Exception(error_msg) | ||
|
Comment on lines
+59
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix bytes formatting in error message. The error message includes Apply this diff to decode the bytes and remove the type ignore: - error_text = await response.aread()
- # TODO: Fix bytes formatting - should decode error_text or use !r
- error_msg = f"Failed to stream results: {response.status_code}, body: {error_text}" # type: ignore[str-bytes-safe] # noqa: E501
- raise Exception(error_msg)
+ error_text = await response.aread()
+ error_msg = f"Failed to stream results: {response.status_code}, body: {error_text.decode('utf-8', errors='replace')}"
+ raise Exception(error_msg)Additional note: As per static analysis, the
🧰 Tools🪛 Ruff (0.14.5)60-60: Unused Remove unused (RUF100) 61-61: Create your own exception (TRY002) 🤖 Prompt for AI Agents |
||
|
|
||
| response_text = await response.aread() | ||
| return self._parse_sse_result(response_text.decode()) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.