ci: pre-commit hooks for static analysis (#510)

* chore: add pre-commit for ruff & black * docs: add pre-commit setup instructions to CONTRIBUTING.md * chore: update ruff configuration in pyproject.toml * when you run pre-commit run --all-files, it should actually format the files instead of just reporting what would change. After the formatting is done, you should see the changes in git status and be able to commit them. * Remove black; will fight over eachother with ruff * run ruff on pre-existing issues * chore(pyproject.toml): remove black, update dirctories * ci: remove black-formatter.yaml * ci: add static-analysis.yaml running pre-commit hook * build: add ruff to development dependencies
AgentOps-AI · Nov 16, 2024 · 80d4bf5 · 80d4bf5
1 parent a0d85c5
commit 80d4bf5
Show file tree

Hide file tree

Showing 42 changed files with 288 additions and 479 deletions.
diff --git a/.github/workflows/black-formatter.yml b/.github/workflows/black-formatter.yml
diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml
@@ -0,0 +1,62 @@
+name: Static analysis
+
+# on PR and push to main
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+    paths:
+      - '**/*.py'
+
+permissions:
+  contents: read
+
+# Limit concurrency by workflow/branch combination.
+#
+# For pull request builds, pushing additional changes to the
+# branch will cancel prior in-progress and pending builds.
+#
+# For builds triggered on a branch push, additional changes
+# will wait for prior builds to complete before starting.
+#
+# https://docs.github.com/en/actions/using-jobs/using-concurrency
+concurrency:
+  group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
+  cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+jobs:
+  pre-commit-checks:
+    name: Pre-commit checks
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          persist-credentials: false
+          fetch-depth: 0
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        id: setup_python
+        with:
+          python-version: "3.11.10"
+
+      - name: UV Cache
+        # Manually cache the uv cache directory
+        # until setup-python supports it:
+        # https://github.com/actions/setup-python/issues/822
+        uses: actions/cache@v4
+        id: cache-uv
+        with:
+          path: ~/.cache/uv
+          key: uvcache-${{ runner.os }}-${{ steps.setup_python.outputs.python-version }}-${{ hashFiles('pyproject.toml') }}
+
+      - name: Install packages
+        run: |
+          python -m pip install -U uv pre-commit
+          uv pip install --upgrade --system -e .[dev]
+
+      - name: Run pre-commit
+        run: |
+          pre-commit run --show-diff-on-failure --color=always --all-files
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -0,0 +1,7 @@
+repos:
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: "v0.2.1"
+    hooks:
+      - id: ruff
+        args: [--fix, --exit-non-zero-on-fix]
+      - id: ruff-format
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -77,6 +77,18 @@ Even if you're not ready to contribute code, we'd love to hear your thoughts. Dr
    .\venv\Scripts\activate   # Windows
    ```
 
+3. **Pre-commit Setup**:
+   We use pre-commit hooks to automatically format and lint code. Set them up with:
+   ```bash
+   pip install pre-commit
+   pre-commit install
+   ```
+
+   That's it! The hooks will run automatically when you commit. To manually check all files:
+   ```bash
+   pre-commit run --all-files
+   ```
+
 ## Testing
 
 We use a comprehensive testing stack to ensure code quality and reliability. Our testing framework includes pytest and several specialized testing tools.

diff --git a/agentops/cli.py b/agentops/cli.py
@@ -6,9 +6,7 @@ def main():
     parser = argparse.ArgumentParser(description="AgentOps CLI")
     subparsers = parser.add_subparsers(dest="command")
 
-    timetravel_parser = subparsers.add_parser(
-        "timetravel", help="Time Travel Debugging commands", aliases=["tt"]
-    )
+    timetravel_parser = subparsers.add_parser("timetravel", help="Time Travel Debugging commands", aliases=["tt"])
     timetravel_parser.add_argument(
         "branch_name",
         type=str,

diff --git a/agentops/client.py b/agentops/client.py
@@ -44,10 +44,7 @@ def __init__(self):
             api_key=os.environ.get("AGENTOPS_API_KEY"),
             parent_key=os.environ.get("AGENTOPS_PARENT_KEY"),
             endpoint=os.environ.get("AGENTOPS_API_ENDPOINT"),
-            env_data_opt_out=os.environ.get(
-                "AGENTOPS_ENV_DATA_OPT_OUT", "False"
-            ).lower()
-            == "true",
+            env_data_opt_out=os.environ.get("AGENTOPS_ENV_DATA_OPT_OUT", "False").lower() == "true",
         )
 
     def configure(
@@ -106,9 +103,7 @@ def initialize(self) -> Union[Session, None]:
 
         if session:
             for agent_args in self._pre_init_queue["agents"]:
-                session.create_agent(
-                    name=agent_args["name"], agent_id=agent_args["agent_id"]
-                )
+                session.create_agent(name=agent_args["name"], agent_id=agent_args["agent_id"])
             self._pre_init_queue["agents"] = []
 
         return session
@@ -141,9 +136,7 @@ def add_tags(self, tags: List[str]) -> None:
 
         session = self._safe_get_session()
         if session is None:
-            return logger.warning(
-                "Could not add tags. Start a session by calling agentops.start_session()."
-            )
+            return logger.warning("Could not add tags. Start a session by calling agentops.start_session().")
 
         session.add_tags(tags=tags)
 
@@ -162,9 +155,7 @@ def set_tags(self, tags: List[str]) -> None:
         session = self._safe_get_session()
 
         if session is None:
-            return logger.warning(
-                "Could not set tags. Start a session by calling agentops.start_session()."
-            )
+            return logger.warning("Could not set tags. Start a session by calling agentops.start_session().")
         else:
             session.set_tags(tags=tags)
 
@@ -198,9 +189,7 @@ def record(self, event: Union[Event, ErrorEvent]) -> None:
 
         session = self._safe_get_session()
         if session is None:
-            return logger.error(
-                "Could not record event. Start a session by calling agentops.start_session()."
-            )
+            return logger.error("Could not record event. Start a session by calling agentops.start_session().")
         session.record(event)
 
     def start_session(
@@ -244,9 +233,7 @@ def start_session(
 
         if self._pre_init_queue["agents"] and len(self._pre_init_queue["agents"]) > 0:
             for agent_args in self._pre_init_queue["agents"]:
-                session.create_agent(
-                    name=agent_args["name"], agent_id=agent_args["agent_id"]
-                )
+                session.create_agent(name=agent_args["name"], agent_id=agent_args["agent_id"])
             self._pre_init_queue["agents"] = []
 
         self._sessions.append(session)
@@ -277,9 +264,7 @@ def end_session(
         if is_auto_end and self._config.skip_auto_end_session:
             return
 
-        token_cost = session.end_session(
-            end_state=end_state, end_state_reason=end_state_reason, video=video
-        )
+        token_cost = session.end_session(end_state=end_state, end_state_reason=end_state_reason, video=video)
 
         return token_cost
 
@@ -299,9 +284,7 @@ def create_agent(
             # if no session passed, assume single session
             session = self._safe_get_session()
             if session is None:
-                self._pre_init_queue["agents"].append(
-                    {"name": name, "agent_id": agent_id}
-                )
+                self._pre_init_queue["agents"].append({"name": name, "agent_id": agent_id})
             else:
                 session.create_agent(name=name, agent_id=agent_id)
 
@@ -326,9 +309,7 @@ def signal_handler(signum, frame):
             """
             signal_name = "SIGINT" if signum == signal.SIGINT else "SIGTERM"
             logger.info("%s detected. Ending session...", signal_name)
-            self.end_session(
-                end_state="Fail", end_state_reason=f"Signal {signal_name} detected"
-            )
+            self.end_session(end_state="Fail", end_state_reason=f"Signal {signal_name} detected")
             sys.exit(0)
 
         def handle_exception(exc_type, exc_value, exc_traceback):
@@ -341,9 +322,7 @@ def handle_exception(exc_type, exc_value, exc_traceback):
                 exc_traceback (TracebackType): A traceback object encapsulating the call stack at the
                                             point where the exception originally occurred.
             """
-            formatted_traceback = "".join(
-                traceback.format_exception(exc_type, exc_value, exc_traceback)
-            )
+            formatted_traceback = "".join(traceback.format_exception(exc_type, exc_value, exc_traceback))
 
             for session in self._sessions:
                 session.end_session(
@@ -376,13 +355,7 @@ def add_pre_init_warning(self, message: str):
     # replaces the session currently stored with a specific session_id, with a new session
     def _update_session(self, session: Session):
         self._sessions[
-            self._sessions.index(
-                [
-                    sess
-                    for sess in self._sessions
-                    if sess.session_id == session.session_id
-                ][0]
-            )
+            self._sessions.index([sess for sess in self._sessions if sess.session_id == session.session_id][0])
         ] = session
 
     def _safe_get_session(self) -> Optional[Session]:
@@ -392,9 +365,7 @@ def _safe_get_session(self) -> Optional[Session]:
             return self._sessions[0]
 
         if len(self._sessions) > 1:
-            calling_function = inspect.stack()[
-                2
-            ].function  # Using index 2 because we have a wrapper at index 1
+            calling_function = inspect.stack()[2].function  # Using index 2 because we have a wrapper at index 1
             return logger.warning(
                 f"Multiple sessions detected. You must use session.{calling_function}(). More info: https://docs.agentops.ai/v1/concepts/core-concepts#session-management"
             )

diff --git a/agentops/decorators.py b/agentops/decorators.py
@@ -45,9 +45,7 @@ async def async_wrapper(*args, session: Optional[Session] = None, **kwargs):
                 arg_names = list(func_args.keys())
                 # Get default values
                 arg_values = {
-                    name: func_args[name].default
-                    for name in arg_names
-                    if func_args[name].default is not inspect._empty
+                    name: func_args[name].default for name in arg_names if func_args[name].default is not inspect._empty
                 }
                 # Update with positional arguments
                 arg_values.update(dict(zip(arg_names, args)))
@@ -111,9 +109,7 @@ def sync_wrapper(*args, session: Optional[Session] = None, **kwargs):
                 arg_names = list(func_args.keys())
                 # Get default values
                 arg_values = {
-                    name: func_args[name].default
-                    for name in arg_names
-                    if func_args[name].default is not inspect._empty
+                    name: func_args[name].default for name in arg_names if func_args[name].default is not inspect._empty
                 }
                 # Update with positional arguments
                 arg_values.update(dict(zip(arg_names, args)))
@@ -191,9 +187,7 @@ async def async_wrapper(*args, session: Optional[Session] = None, **kwargs):
                 arg_names = list(func_args.keys())
                 # Get default values
                 arg_values = {
-                    name: func_args[name].default
-                    for name in arg_names
-                    if func_args[name].default is not inspect._empty
+                    name: func_args[name].default for name in arg_names if func_args[name].default is not inspect._empty
                 }
                 # Update with positional arguments
                 arg_values.update(dict(zip(arg_names, args)))
@@ -257,9 +251,7 @@ def sync_wrapper(*args, session: Optional[Session] = None, **kwargs):
                 arg_names = list(func_args.keys())
                 # Get default values
                 arg_values = {
-                    name: func_args[name].default
-                    for name in arg_names
-                    if func_args[name].default is not inspect._empty
+                    name: func_args[name].default for name in arg_names if func_args[name].default is not inspect._empty
                 }
                 # Update with positional arguments
                 arg_values.update(dict(zip(arg_names, args)))
@@ -338,20 +330,17 @@ def new_init(self, *args, **kwargs):
                         session=session,
                     )
                 except AttributeError as e:
-                    Client().add_pre_init_warning(
-                        f"Failed to track an agent {name} with the @track_agent decorator."
-                    )
-                    logger.warning(
-                        "Failed to track an agent with the @track_agent decorator."
-                    )
+                    Client().add_pre_init_warning(f"Failed to track an agent {name} with the @track_agent decorator.")
+                    logger.warning("Failed to track an agent with the @track_agent decorator.")
                     original_init(self, *args, **kwargs)
 
             obj.__init__ = new_init
 
         elif inspect.isfunction(obj):
             obj.agent_ops_agent_id = str(uuid4())  # type: ignore
             Client().create_agent(
-                name=obj.agent_ops_agent_name, agent_id=obj.agent_ops_agent_id  # type: ignore
+                name=obj.agent_ops_agent_name,
+                agent_id=obj.agent_ops_agent_id,  # type: ignore
             )
 
         else:

diff --git a/agentops/helpers.py b/agentops/helpers.py
@@ -9,7 +9,6 @@
 
 from .log_config import logger
 from uuid import UUID
-from importlib.metadata import version
 
 
 def get_ISO_time():
@@ -38,7 +37,9 @@ def filter_dict(obj):
                 k: (
                     filter_dict(v)
                     if isinstance(v, (dict, list)) or is_jsonable(v)
-                    else str(v) if isinstance(v, UUID) else ""
+                    else str(v)
+                    if isinstance(v, UUID)
+                    else ""
                 )
                 for k, v in obj.items()
             }
@@ -47,7 +48,9 @@ def filter_dict(obj):
                 (
                     filter_dict(x)
                     if isinstance(x, (dict, list)) or is_jsonable(x)
-                    else str(x) if isinstance(x, UUID) else ""
+                    else str(x)
+                    if isinstance(x, UUID)
+                    else ""
                 )
                 for x in obj
             ]
@@ -85,9 +88,7 @@ def remove_unwanted_items(value):
         """Recursively remove self key and None/... values from dictionaries so they aren't serialized"""
         if isinstance(value, dict):
             return {
-                k: remove_unwanted_items(v)
-                for k, v in value.items()
-                if v is not None and v is not ... and k != "self"
+                k: remove_unwanted_items(v) for k, v in value.items() if v is not None and v is not ... and k != "self"
             }
         elif isinstance(value, list):
             return [remove_unwanted_items(item) for item in value]
@@ -106,9 +107,7 @@ def check_call_stack_for_agent_id() -> Union[UUID, None]:
             # We stop looking up the stack at main because after that we see global variables
             if var == "__main__":
                 return None
-            if hasattr(var, "agent_ops_agent_id") and getattr(
-                var, "agent_ops_agent_id"
-            ):
+            if hasattr(var, "agent_ops_agent_id") and getattr(var, "agent_ops_agent_id"):
                 logger.debug(
                     "LLM call from agent named: %s",
                     getattr(var, "agent_ops_agent_name"),
@@ -141,7 +140,7 @@ def check_agentops_update():
 
             if not latest_version == current_version:
                 logger.warning(
-                    f" WARNING: agentops is out of date. Please update with the command: 'pip install --upgrade agentops'"
+                    " WARNING: agentops is out of date. Please update with the command: 'pip install --upgrade agentops'"
                 )
     except Exception as e:
         logger.debug(f"Failed to check for updates: {e}")