fix: address code quality review comments

- Remove unused current_branch variable in detect_pr_from_branch
- Add explicit return statements after pytest.skip() in decorators
- Add diagnostic logging to empty except clauses
- Add type annotations to production test fixtures and decorators

Author: vdavez

scripts/pr_review.py

@@ -142,8 +142,8 @@ def detect_pr_from_github_actions() -> PRInfo | None:
                             url=pr.get("html_url"),
                             state=pr.get("state"),
                         )
-            except Exception:
-                pass
+            except Exception as e:
+                print(f"Warning: Could not read/parse GitHub event file: {e}", file=sys.stderr)
 
     if pr_number:
         try:
@@ -195,9 +195,10 @@ def get_pr_info_from_gh_cli(pr_number: int | None = None) -> PRInfo | None:
             except (json.JSONDecodeError, KeyError):
                 pass
     except FileNotFoundError:
+        # gh CLI not available - this is expected in some environments
         pass
-    except Exception:
-        pass
+    except Exception as e:
+        print(f"Warning: Error getting PR info from gh CLI: {e}", file=sys.stderr)
 
     return None
 
@@ -214,11 +215,10 @@ def detect_pr_from_branch() -> PRInfo | None:
             check=False,
         )
         if result.returncode == 0:
-            current_branch = result.stdout.strip()
             # Try to get PR info for this branch
             return get_pr_info_from_gh_cli(None)
-    except Exception:
-        pass
+    except Exception as e:
+        print(f"Warning: Error detecting PR from branch: {e}", file=sys.stderr)
 
     return None
 

tests/production/conftest.py

@@ -1,11 +1,14 @@
 """Pytest configuration and fixtures for production smoke tests"""
 
 import os
+from collections.abc import Callable
 from functools import wraps
+from typing import Any
 
 import pytest
 from dotenv import load_dotenv
 
+from tango import TangoClient
 from tango.exceptions import TangoAuthError, TangoRateLimitError
 
 # Load environment variables from .env file if it exists
@@ -16,21 +19,19 @@
 
 
 @pytest.fixture
-def production_client():
+def production_client() -> TangoClient:
     """
     Create TangoClient for production smoke tests
 
     Requires TANGO_API_KEY environment variable to be set.
     """
-    from tango import TangoClient
-
     if not API_KEY:
         pytest.skip("TANGO_API_KEY environment variable required for production tests")
 
     return TangoClient(api_key=API_KEY)
 
 
-def handle_auth_error(func):
+def handle_auth_error(func: Callable[..., Any]) -> Callable[..., Any]:
     """Decorator to handle authentication errors in production tests
 
     Skips the test if authentication fails, which allows the test suite
@@ -44,16 +45,17 @@ def test_something(production_client):
     """
 
     @wraps(func)
-    def wrapper(*args, **kwargs):
+    def wrapper(*args: Any, **kwargs: Any) -> Any:
         try:
             return func(*args, **kwargs)
         except TangoAuthError as e:
             pytest.skip(f"Authentication failed: {e}")
+            return  # type: ignore[unreachable]  # Explicit return for static analysis
 
     return wrapper
 
 
-def handle_rate_limit(func):
+def handle_rate_limit(func: Callable[..., Any]) -> Callable[..., Any]:
     """Decorator to handle rate limit errors in production tests
 
     Skips the test if rate limit is exceeded, which allows the test suite
@@ -67,10 +69,11 @@ def test_something(production_client):
     """
 
     @wraps(func)
-    def wrapper(*args, **kwargs):
+    def wrapper(*args: Any, **kwargs: Any) -> Any:
         try:
             return func(*args, **kwargs)
         except TangoRateLimitError as e:
             pytest.skip(f"Rate limit exceeded: {e}")
+            return  # type: ignore[unreachable]  # Explicit return for static analysis
 
     return wrapper