Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions posthog/ai/prompts.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ class Prompts:
# Or with direct options (no PostHog client needed)
prompts = Prompts(personal_api_key='phx_xxx', host='https://us.posthog.com')

# With error tracking: prompt fetch failures are reported to PostHog
prompts = Prompts(posthog, capture_errors=True)

# Fetch with caching and fallback
template = prompts.get('support-system-prompt', fallback='You are a helpful assistant.')

Expand All @@ -74,6 +77,7 @@ def __init__(
personal_api_key: Optional[str] = None,
host: Optional[str] = None,
default_cache_ttl_seconds: Optional[int] = None,
capture_errors: bool = False,
):
"""
Initialize Prompts.
Expand All @@ -83,11 +87,15 @@ def __init__(
personal_api_key: Direct API key (optional if posthog provided)
host: PostHog host (defaults to app endpoint)
default_cache_ttl_seconds: Default cache TTL (defaults to 300)
capture_errors: If True and a PostHog client is provided, prompt fetch
failures are reported to PostHog error tracking via capture_exception().
"""
self._default_cache_ttl_seconds = (
default_cache_ttl_seconds or DEFAULT_CACHE_TTL_SECONDS
)
self._cache: Dict[str, CachedPrompt] = {}
self._client = posthog if posthog is not None else None
self._capture_errors = capture_errors

Comment on lines +97 to 99
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unchecked capture_exception call

_maybe_capture_error() assumes the provided posthog object has a capture_exception method. With capture_errors=True, passing a PostHog client version (or a stub) that lacks capture_exception will raise AttributeError, which then gets swallowed by the broad except Exception, meaning errors silently won’t be reported (and it’s hard to diagnose). Consider guarding the call (e.g., hasattr(self._client, "capture_exception")) and/or logging at least once at WARNING when the method is missing so users know why capture isn’t happening.

Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/ai/prompts.py
Line: 97:99

Comment:
**Unchecked capture_exception call**

`_maybe_capture_error()` assumes the provided `posthog` object has a `capture_exception` method. With `capture_errors=True`, passing a PostHog client version (or a stub) that lacks `capture_exception` will raise `AttributeError`, which then gets swallowed by the broad `except Exception`, meaning errors silently won’t be reported (and it’s hard to diagnose). Consider guarding the call (e.g., `hasattr(self._client, "capture_exception")`) and/or logging at least once at WARNING when the method is missing so users know why capture isn’t happening.

How can I resolve this? If you propose a fix, please make it concise.

if posthog is not None:
self._personal_api_key = getattr(posthog, "personal_api_key", None) or ""
Expand Down Expand Up @@ -152,6 +160,8 @@ def get(
return prompt

except Exception as error:
self._maybe_capture_error(error)

# Fallback order:
# 1. Return stale cache (with warning)
if cached is not None:
Expand Down Expand Up @@ -211,6 +221,15 @@ def clear_cache(self, name: Optional[str] = None) -> None:
else:
self._cache.clear()

def _maybe_capture_error(self, error: Exception) -> None:
"""Report a prompt fetch error to PostHog error tracking if enabled."""
if not self._capture_errors or self._client is None:
return
try:
self._client.capture_exception(error)
except Exception:
log.debug("[PostHog Prompts] Failed to capture exception to error tracking")

def _fetch_prompt_from_api(self, name: str) -> str:
"""
Fetch prompt from PostHog API.
Expand Down
115 changes: 115 additions & 0 deletions posthog/test/ai/test_prompts.py
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,121 @@ def test_handle_variables_with_dots(self):
self.assertEqual(result, "Company: Acme")


class TestPromptsCaptureErrors(TestPrompts):
"""Tests for the capture_errors option."""

@patch("posthog.ai.prompts._get_session")
def test_capture_exception_called_on_fetch_failure_with_fallback(
Comment on lines +512 to +516
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are not parameterized

The new TestPromptsCaptureErrors cases are all the same shape (setup fetch failure/success → call get() → assert on capture_exception). This increases repetition and makes it easier to miss scenarios when adding new branches. The repo preference is parameterized tests; these can be collapsed into a single parameterized test that covers (fallback vs stale cache vs re-raise vs success) and toggles (capture_errors on/off, client present/absent).

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/test/ai/test_prompts.py
Line: 512:516

Comment:
**Tests are not parameterized**

The new `TestPromptsCaptureErrors` cases are all the same shape (setup fetch failure/success → call `get()` → assert on `capture_exception`). This increases repetition and makes it easier to miss scenarios when adding new branches. The repo preference is parameterized tests; these can be collapsed into a single parameterized test that covers (fallback vs stale cache vs re-raise vs success) and toggles (`capture_errors` on/off, client present/absent).

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

self, mock_get_session
):
"""Should call capture_exception on fetch failure when capture_errors=True."""
mock_get = mock_get_session.return_value.get
mock_get.side_effect = Exception("Network error")

posthog = self.create_mock_posthog()
prompts = Prompts(posthog, capture_errors=True)

result = prompts.get("test-prompt", fallback="fallback prompt")

self.assertEqual(result, "fallback prompt")
posthog.capture_exception.assert_called_once()
captured_exc = posthog.capture_exception.call_args[0][0]
self.assertIn("Network error", str(captured_exc))

@patch("posthog.ai.prompts._get_session")
@patch("posthog.ai.prompts.time.time")
def test_capture_exception_called_on_fetch_failure_with_stale_cache(
self, mock_time, mock_get_session
):
"""Should call capture_exception when falling back to stale cache."""
mock_get = mock_get_session.return_value.get
mock_get.side_effect = [
MockResponse(json_data=self.mock_prompt_response),
Exception("Network error"),
]
mock_time.return_value = 1000.0

posthog = self.create_mock_posthog()
prompts = Prompts(posthog, capture_errors=True)

# First call populates cache
prompts.get("test-prompt", cache_ttl_seconds=60)

# Expire cache
mock_time.return_value = 1061.0

# Second call falls back to stale cache
result = prompts.get("test-prompt", cache_ttl_seconds=60)
self.assertEqual(result, self.mock_prompt_response["prompt"])
posthog.capture_exception.assert_called_once()

@patch("posthog.ai.prompts._get_session")
def test_capture_exception_called_when_error_is_raised(self, mock_get_session):
"""Should call capture_exception even when the error is re-raised (no fallback, no cache)."""
mock_get = mock_get_session.return_value.get
mock_get.side_effect = Exception("Network error")

posthog = self.create_mock_posthog()
prompts = Prompts(posthog, capture_errors=True)

with self.assertRaises(Exception):
prompts.get("test-prompt")

posthog.capture_exception.assert_called_once()

@patch("posthog.ai.prompts._get_session")
def test_no_capture_exception_when_capture_errors_is_false(self, mock_get_session):
"""Should NOT call capture_exception when capture_errors=False (default)."""
mock_get = mock_get_session.return_value.get
mock_get.side_effect = Exception("Network error")

posthog = self.create_mock_posthog()
prompts = Prompts(posthog)

prompts.get("test-prompt", fallback="fallback prompt")

posthog.capture_exception.assert_not_called()

@patch("posthog.ai.prompts._get_session")
def test_no_capture_exception_without_client(self, mock_get_session):
"""Should not error when capture_errors=True but no client provided."""
mock_get = mock_get_session.return_value.get
mock_get.side_effect = Exception("Network error")

prompts = Prompts(personal_api_key="phx_test_key", capture_errors=True)

result = prompts.get("test-prompt", fallback="fallback prompt")

self.assertEqual(result, "fallback prompt")

@patch("posthog.ai.prompts._get_session")
def test_no_capture_exception_on_successful_fetch(self, mock_get_session):
"""Should NOT call capture_exception on successful fetch."""
mock_get = mock_get_session.return_value.get
mock_get.return_value = MockResponse(json_data=self.mock_prompt_response)

posthog = self.create_mock_posthog()
prompts = Prompts(posthog, capture_errors=True)

prompts.get("test-prompt")

posthog.capture_exception.assert_not_called()

@patch("posthog.ai.prompts._get_session")
def test_capture_exception_failure_does_not_affect_fallback(self, mock_get_session):
"""If capture_exception itself throws, the fallback should still be returned."""
mock_get = mock_get_session.return_value.get
mock_get.side_effect = Exception("Network error")

posthog = self.create_mock_posthog()
posthog.capture_exception.side_effect = Exception("capture failed")
prompts = Prompts(posthog, capture_errors=True)

result = prompts.get("test-prompt", fallback="fallback prompt")

self.assertEqual(result, "fallback prompt")


class TestPromptsClearCache(TestPrompts):
"""Tests for the Prompts.clear_cache() method."""

Expand Down
2 changes: 1 addition & 1 deletion posthog/version.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
VERSION = "7.8.3"
VERSION = "7.8.4"

if __name__ == "__main__":
print(VERSION, end="") # noqa: T201
Loading