fix/ph-job-ids
safe9a1a984 · incrementalPR #364reviewed 2026-07-31 01:18 UTC0H · 0M · 1L · 2I- Purpose
- Stage 3 of 3 in the PostHog groundwork series (BAT-286): fix the PostHog identity join keys so server-emitted cfe_job events land on the correct person+org rather than a phantom person with no org.
- Goal
- Tighten the analytics server API type safety and consolidate the join-invariant documentation so it lives in the JSDoc on trackServer rather than being duplicated at each call site.
- Sub-goals
- Add PostHogGroupType to prevent silent phantom-join from a mistyped group key
- Move the distinctId/groups join rationale into trackServer JSDoc
- Add org_id to cfe_job.attempt properties for cross-deploy-boundary backward compat
- What
- packages/analytics/src/server.ts: narrowed groups param from Record<string,string> to Partial<Record<PostHogGroupType,string>>, added PostHogGroupType type alias, improved JSDoc. cfe-jobs.handler.ts: condensed call-site comments (rationale now lives in JSDoc), added org_id property with backward-compat explanation.
- Why
- The prior PR (PR 2/3) established the correct distinctId+groups keys on the call sites; this commit tightens the type surface so future call sites can't accidentally use an unregistered group key that PostHog silently drops at ingest, and cleans up the now-redundant inline comments.
- Areas
- packages/analytics+33−8apps/platform/src/api/handlers+12−4
- Blast
- 2 files, +45/-12 lines. Type-level change only — no runtime behavior affected. Both changed files are internal to the analytics/platform packages.
Findings · 3
conventions1
Trailing sentence in org_id comment explains convention rather than constraint
apps/platform/src/api/handlers/cfe-jobs.handler.ts:298
The comment reads: '…dropping it would leave no single field that segments by org across the deploy boundary. Also the repo-wide convention on ~8 event types.' The first part is a non-obvious invariant (deploy-boundary backward compat) — that warrants a comment. The second sentence ('Also the repo-wide convention…') leans toward describing what rather than why. Verified accurate (8 occurrences in events.ts), but borderline by CLAUDE.md conventions.
tests1
PostHogGroupType constraint is compile-time only — no runtime test covers misuse
packages/analytics/src/server.ts:60
The type narrows groups to `{ organization?: string }` at compile time. Correct and sufficient for now (only 2 call sites, both verified). Not a blocking gap — if the group type list grows or if `trackServer` is re-exported with a looser wrapper, a small unit test asserting valid/invalid group keys would catch drift. Non-blocking info finding.
improvement1
PostHogGroupType is unexported — call sites can't reference it directly
packages/analytics/src/server.ts:20
External packages (e.g. apps/platform handlers) pass `{ organization: orgPublicId }` as a literal, which TypeScript checks structurally — so there is no compile error today. But if a new call site wants to express the type explicitly (e.g. a helper that builds the groups map), they'd need `Parameters<typeof trackServer>[0]['groups']` which is awkward. Exporting `PostHogGroupType` makes the constraint visible and reusable without friction. Low urgency: only 2 call sites exist currently, both literals.