feat: Add context helpers to Client#681
Merged
Merged
Conversation
Contributor
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
posthog/client.py:488-523
**`Client.scoped()` duplicates `contexts.scoped()`**
`Client.scoped()` is a near-verbatim copy of `contexts.scoped()` — the only difference is `self.new_context()` instead of `new_context()`, which threads `client=self` through to `new_context()`. Adding a `client` parameter to `contexts.scoped()` would let `Client.scoped()` become a one-liner delegate (`return _context_scoped(fresh=fresh, capture_exceptions=capture_exceptions, client=self)`) and remove the ~18 lines of duplicated async/sync branching, in line with the OnceAndOnlyOnce simplicity rule.
### Issue 2 of 3
posthog/test/test_client.py:2530-2553
**No async path tested for `Client.scoped()`**
`Client.scoped()` branches on `inspect.iscoroutinefunction` and generates a distinct `async_wrapper`, but only the synchronous path is exercised here. A test decorating an `async def` function and running it with `asyncio.run()` (mirroring the approach in `test_contexts.py`) would cover the async branch.
### Issue 3 of 3
posthog/test/test_client.py:2496-2553
**Two structurally identical tests could be parameterised**
`test_client_context_helpers_apply_to_capture` and `test_client_scoped_context_helpers_apply_to_capture` assert the same properties on the captured event — they differ only in how the context is entered (`with client.new_context(fresh=True)` vs `@client.scoped(fresh=True)`). Per the project's test style, parameterising over the context-creation mechanism would express both cases without repeating the assertion block.
Reviews (1): Last reviewed commit: "fix: Add context helpers to Client" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
posthog-python Compliance ReportDate: 2026-06-19 14:58:07 UTC ✅ All Tests Passed!45/45 tests passed Capture Tests✅ 29/29 tests passed View Details
Feature_Flags Tests✅ 16/16 tests passed View Details
|
marandaneto
changed the title
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
posthog/client.py:503-514
**`name` vs `key` parameter name inconsistency**
The module-level `tag(key, value)` in `contexts.py` documents its first argument as `key`, but `Client.tag` names it `name`. Positional callers are unaffected, but anyone who switches between the module-level and client APIs and tries to call `client.tag(key="foo", value="bar")` will get a `TypeError`. Aligning the parameter name to `key` would keep the two APIs consistent.
### Issue 2 of 2
posthog/client.py:516-562
**Setters added, getter counterparts omitted**
The PR exposes `set_context_session`, `set_context_device_id`, and `identify_context` on `Client`, but not their read-back counterparts `get_context_session_id`, `get_context_distinct_id`, and `get_context_device_id`. `get_tags` was included, so the setter/getter pairing is asymmetric. Custom-client users who need to read the current session or device ID back are forced to reach for the module-level functions, which is exactly the ergonomic gap this PR aims to close.
Reviews (2): Last reviewed commit: "mark client context helpers as minor" | Re-trigger Greptile |
…thods-289 # Conflicts: # posthog/contexts.py # references/public_api_snapshot.txt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 Motivation and Context
Closes #289.
Custom
Clientinstances already hadnew_context(), but context helpers such as tagging, context identity, session IDs, device IDs, and scoped decorators were only available through module-level helpers. This made custom-client usage less ergonomic and inconsistent with the module-level API.This PR adds thin
Clientwrappers for the existing context helpers without changing the contextvars storage model.💚 How did you test it?
uvx ruff format posthog/client.py posthog/test/test_client.pyuv run --extra dev python .github/scripts/check_public_api.py --writeuv run --extra dev python .github/scripts/check_public_api.pyuv run --extra test pytest -q posthog/test/test_contexts.py posthog/test/test_client.pygit diff --check📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
A Pi worker agent implemented the user-directed fix for issue #289. The change intentionally keeps context storage unchanged and exposes only thin custom-client wrappers around the existing context helper APIs;
Client.scoped()usesClient.new_context()so exception capture remains bound to the custom client.