fix(ai): let Prompts.get_all() batch without a label - #964
Draft
posthog[bot] wants to merge 1 commit into
Draft
posthog[bot] wants to merge 1 commit into
posthog[bot] wants to merge 1 commit into
Conversation
`get_all` took a required keyword-only `label`, so an app that does not use labels had no batch entry point and fell back to one `get()` request per prompt on every cache cycle. Passing `label=None` sent the literal string "None" over the wire. `label` is now optional. Without it the list endpoint returns the latest version of every prompt, cached under the same key `get(name)` reads. The old-server label guards stay on the labeled path, where the label is the thing an old server can ignore. Refs #959 Generated-By: PostHog Desktop Task-Id: afd5da60-e8ce-4008-bb05-ea28a501b088
Contributor
Author
🦔 PostHog Review reviewed this pull requestFound 0 must fix, 1 should fix, 0 consider. Published 1 finding (view the review). Resolved comments: 1 left for you |
Contributor
posthog-python Compliance ReportDate: 2026-09-16 02:36:58 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
Contributor
Author
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Comment on lines
+691
to
+694
| params = {"token": self._project_api_key, "content": "full"} | ||
| if label is not None: | ||
| params["label"] = label | ||
| query = urllib.parse.urlencode(params) |
Contributor
Author
There was a problem hiding this comment.
Unlabeled batch fetches lose prompt-fetch usage events
Issue description
The new unlabeled path calls the list endpoint without label. The deployed server emits $llm_prompt_fetched for labeled list calls only. Repeated get(name) calls emit one event per prompt. An app that switches to get_all() therefore stops reporting these fetches. Prompt-only teams can also disappear from AI observability usage reports. The required server fix remains an open draft.
Why we think it's a valid issue
- Checked: the new unlabeled request build at posthog/ai/prompts.py:691-694, the whole posthog-python tree for any client-side fetch event, the server
list()handler on PostHog/posthog master, the ingestion billable-event list, and the AI observability usage report task. - Found: the server tracks list fetches only under a label. On master,
list()callsself._track_labeled_list_fetches(prompts, label)inside anif label:guard (posthog/api/llm_prompt.py:685-689), while the single-prompt action callsself._track_prompt_fetch(prompt)unconditionally (posthog/api/llm_prompt.py:358). The premise holds: the path this PR adds produces no$llm_prompt_fetchedevent, and the path it replaces produces one per prompt per cache cycle. - Found: the gap is still open. The most recent commit on posthog/api/llm_prompt.py is
7d776ae0 feat(aio): report prompt fetches on labeled list calls (#98183). feat(aio): report prompt fetches on unlabeled list calls posthog#100690 reportsstate: OPEN,isDraft: true,mergedAt: null, so the unlabeled tracking is not deployed. - Found: the SDK sends nothing of its own to compensate. A search for
llm_prompt_fetchedacross posthog-python returns no match, and_maybe_capture_errorreports exceptions only. - Found: the loss reaches team selection, not only a count.
AI_OBSERVABILITY_REPORT_TRIGGER_EVENTSincludes$llm_prompt_fetched(posthog/tasks/ai_observability_usage_report.py:47) and feedsget_teams_with_ai_events(...)at line 911. A team that uses prompt management but emits no$ai_*events drops out of the report after it moves to the unlabeled batch call. - Found: this file already records server-version dependencies, so a rollout note fits the existing convention. The
get_alldocstring names "PostHog releases from before September 2026" for the labeled path (posthog/ai/prompts.py:395-397). - Impact: an app that adopts the new path stops reporting prompt fetches until the server change deploys. That telemetry is what shows rate-limit headroom, which is the exact signal this PR's own motivation rests on.
- Priority: lowered to
should_fix.$llm_prompt_fetchedsits inNON_BILLABLE_EVENTSin nodejs/src/ingestion/common/usage-records/billable-events.ts:20, so no billing and no customer-facing result changes. No code in this repository is wrong, the correction is server-side and takes effect for every client as soon as it deploys, and the requested server contract test belongs in PostHog/posthog. The action for this PR is release coordination plus a note in the changeset, not a merge blocker.
Suggested fix
Deploy PostHog/posthog#100690 before this SDK release. Record this rollout dependency in the changeset. Add a server contract test that proves an API-key unlabeled list request emits one event for each returned prompt.
Prompt to fix with AI (copy-paste)
## Context
@posthog/ai/prompts.py#L691-694
<issue_description>
The new unlabeled path calls the list endpoint without `label`. The deployed server emits `$llm_prompt_fetched` for labeled list calls only. Repeated `get(name)` calls emit one event per prompt. An app that switches to `get_all()` therefore stops reporting these fetches. Prompt-only teams can also disappear from AI observability usage reports. The required server fix remains an open draft.
</issue_description>
<issue_validation>
- **Checked:** the new unlabeled request build at posthog/ai/prompts.py:691-694, the whole posthog-python tree for any client-side fetch event, the server `list()` handler on PostHog/posthog master, the ingestion billable-event list, and the AI observability usage report task.
- **Found:** the server tracks list fetches only under a label. On master, `list()` calls `self._track_labeled_list_fetches(prompts, label)` inside an `if label:` guard (posthog/api/llm_prompt.py:685-689), while the single-prompt action calls `self._track_prompt_fetch(prompt)` unconditionally (posthog/api/llm_prompt.py:358). The premise holds: the path this PR adds produces no `$llm_prompt_fetched` event, and the path it replaces produces one per prompt per cache cycle.
- **Found:** the gap is still open. The most recent commit on posthog/api/llm_prompt.py is `7d776ae0 feat(aio): report prompt fetches on labeled list calls (#98183)`. PostHog/posthog#100690 reports `state: OPEN`, `isDraft: true`, `mergedAt: null`, so the unlabeled tracking is not deployed.
- **Found:** the SDK sends nothing of its own to compensate. A search for `llm_prompt_fetched` across posthog-python returns no match, and `_maybe_capture_error` reports exceptions only.
- **Found:** the loss reaches team selection, not only a count. `AI_OBSERVABILITY_REPORT_TRIGGER_EVENTS` includes `$llm_prompt_fetched` (posthog/tasks/ai_observability_usage_report.py:47) and feeds `get_teams_with_ai_events(...)` at line 911. A team that uses prompt management but emits no `$ai_*` events drops out of the report after it moves to the unlabeled batch call.
- **Found:** this file already records server-version dependencies, so a rollout note fits the existing convention. The `get_all` docstring names "PostHog releases from before September 2026" for the labeled path (posthog/ai/prompts.py:395-397).
- **Impact:** an app that adopts the new path stops reporting prompt fetches until the server change deploys. That telemetry is what shows rate-limit headroom, which is the exact signal this PR's own motivation rests on.
- **Priority:** lowered to `should_fix`. `$llm_prompt_fetched` sits in `NON_BILLABLE_EVENTS` in nodejs/src/ingestion/common/usage-records/billable-events.ts:20, so no billing and no customer-facing result changes. No code in this repository is wrong, the correction is server-side and takes effect for every client as soon as it deploys, and the requested server contract test belongs in PostHog/posthog. The action for this PR is release coordination plus a note in the changeset, not a merge blocker.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Deploy https://github.com/PostHog/posthog/pull/100690 before this SDK release. Record this rollout dependency in the changeset. Add a server contract test that proves an API-key unlabeled list request emits one event for each returned prompt.
</potential_solution>
Contributor
Author
There was a problem hiding this comment.
Escalating: the gap is real, but it closes in the server repository and depends on a release-sequencing call a human must make.
- I confirmed this SDK sends no prompt-fetch event of its own, so the unlabeled batch path reports nothing until the server side changes.
- The server change is still an open draft, so the dependency holds today.
- A human must decide whether to hold this SDK release until that server change deploys, or ship now and accept the gap.
- I did not add the note to the changeset. That file becomes public release notes, and its wording depends on the decision above.
- The requested contract test belongs in the server repository, which I must not change from this pull request.
How this was verified
No code changed, so no lint or tests were run. Checks made instead: read the unlabeled request build and the error-capture helper in the prompts module, searched the whole repository for any client-side prompt-fetch event (no match), and queried the state of the referenced server pull request (still open and draft). Working tree left clean.
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
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
get_all()took a required keyword-onlylabel, so those apps had no batch entry point and fell back to oneget()request per prompt, on every cache cycle. One customer hit the hourly prompt-fetch cap on staging and opened a support ticket with a patch.label=Nonewas not an escape hatch. It went intourlencodeunguarded and reached the server as the literal string"None".GET /api/environments/@current/llm_prompts/returns the latest version of every prompt whenlabelis omitted.Closes #959 (same API shape, filed for exactly this).
Important
This is a public API change, and
CONTRIBUTING.mdasks for the shape to be agreed on the issue before implementation. #959 has no maintainer reply yet, so this PR stays a draft: please confirm the signature on the issue (or here) before merging. The change is additive, so existingget_all(label=...)callers are unaffected.💚 How did you test it?
Unit tests —
uv run pytest posthog/test/ai/test_prompts.py(79 pass). The 8 pre-existingget_alltests pass unchanged, which is the regression check on the labeled path. Two new tests cover the unlabeled path: nolabelparam in the URL, latest versions returned, the cache seeded under the keyget(name)reads, and the error wording.End-to-end against a real HTTP server — a throwaway
http.serverstanding in for the list and single-prompt endpoints, so URL construction, pagination following, auth header and cache keying were exercised without mocks:get_all()over 5 prompts, 2 per pageget(name)straight afterget(name)on a cold cache (the old shape)Edge cases considered: a prompt carrying no label at all, and a prompt whose label points at an earlier version than the latest — both belong in an unlabeled result, so neither may be dropped by the "moved"/"absent" label logic. Pagination links off the configured host, the page-count backstop, and malformed rows leaving the cache untouched are all on the shared path and still covered.
Also run:
ruff format --check .,ruff check .,mypyon the changed module,make public_api_check, and the fullpytestsuite — 2726 pass. Three failures intest_anthropic.py/test_callbacks.pyare pre-existing: they call the live Anthropic/OpenAI APIs and fail identically on a clean checkout ofmain.📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Fully autonomous — no human drove this, so the PR is left unassigned for the owning team to triage.
Written by Claude Opus 5 in PostHog Desktop, from the inbox report linked below.
self._cacheis per-Promptsinstance, so a client built per request starts cold. Both are left alone. Deduplicating the cache key would be wrong — a label can point at a version that is not the latest, so the two keys are not interchangeable — and a process-wide cache is a design call, not a bug fix. Neither is needed for the reported symptom.continueinside the validation loop, which left two separate "keep this row" exits and made the "no row resolved the label" guard dead-but-live. It now scopes the whole label-resolution block under oneif label is not None:, so the loop has a single keep path. A suggestion to widen_row_label_state(row, None)to return"resolved"was rejected: it would put unlabeled semantics into a helper whose only job is comparing against a requested label.count. Real gain only past a page or two, and out of scope here.Created with PostHog Desktop from this inbox report.