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
12 changes: 11 additions & 1 deletion context/skills/audit/references/3-identification.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,13 @@ Read this skill's bundled `identify-users.md` reference once (typically `.claude

Run **one** Grep: `posthog\.init\(|new PostHog\(|posthog\.Posthog\(|Posthog\(` — locate every PostHog initialization across runtimes. Read each file that contains a hit, once. Determine whether both client and server runtimes initialize PostHog, and if so, how distinct_id flows between them.

**Check for a deliberate anonymous-event opt-out before calling a fallback broken.** A server that can't resolve a stable id sometimes emits `$process_person_profile: false` alongside the capture — PostHog's documented mechanism for exactly this situation. With that property set, even a random-UUID `distinct_id` creates no person profile, so it cannot merge with or corrupt person records. Those events are deliberately anonymous, not accidentally orphaned. Read the properties object, not just the `distinct_id` expression, before judging.

Rule:
- If both client and server runtimes call PostHog, the same distinct_id must be used on both sides for the same user.
- pass: server-side captures source the client's distinct_id (cookie, session token, or explicit hand-off).
- error: server-side captures use a different identifier scheme.
- error: server-side captures use a different identifier scheme for the same user, with no hand-off and no anonymous opt-out.
- warning: the server falls back to a random or synthetic id for some captures. If `$process_person_profile: false` accompanies the fallback, the cost is limited to losing that dimension — say so, and do not claim it contaminates person counts, funnels, or retention. Prefer recommending an already-available stable id (org, account, or tenant id — check whether one is in scope at the call site, since it often is) over recommending the capture be dropped.
- Skip (`pass` with details: "single runtime"): only one runtime initializes PostHog.

Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `cross-runtime-distinct-id`, including `file` (path:line of the most relevant init or capture site) and `details` (one-line explanation). Return when the call completes. Do not write the audit report.
Expand All @@ -112,5 +115,12 @@ Rule:
- Skip (`pass` with details: "no logout/account-switch flow found"): no detectable logout/account-switch flow exists.
- note: `posthog.reset(true)` is valid when a completely clean device ID reset is required.

**Verify the location you recommend is safe — a wrong `reset()` placement is worse than a missing one.** A shared sign-out helper (`clearAuthState`, `clearBrowserStorageOnSignOut`, a storage-clearing utility) looks like the ideal single choke point, and recommending one is tempting because it covers every path at once. But these helpers are frequently also invoked when no session ever existed — on initial page load, or from an auth-state listener that fires for anonymous visitors. `posthog.reset()` on that path mints a fresh anonymous ID on **every visit**, which detaches pre-signup pageviews from the account that follows and inflates unique-visitor counts. That is a larger data problem than the identity merge being fixed.

Before naming a location:
- Trace every caller of the helper. If any caller can run without a prior session, the helper is unsafe — say so explicitly in `details` and recommend the individual sign-out call sites instead.
- A safe site runs only on a genuine identity transition: an explicit user sign-out action, or a listener branch guarded by a real sign-out event (e.g. `event === 'SIGNED_OUT'`) rather than merely "no session present".
- Enumerate every sign-out path you found, including forced sign-outs routed through an auth listener. Recommending only the obvious menu-driven `signOut()` methods leaves session-revocation and account-deletion paths unreset.

Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `identify-reset-on-logout`, including `file` (path:line of the most relevant logout or reset site) and `details` (one-line explanation). Return when the call completes. Do not write the audit report.
```
33 changes: 25 additions & 8 deletions context/skills/audit/references/4-event-capture.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,19 @@ Read this skill's bundled `best-practices.md` reference once (typically `.claude

Run **one** Grep: `posthog\.capture\(`. Read each file that contains a hit, once. Inspect the first argument of every capture() call.

**Resolve the name before judging it.** A first argument that isn't a bare string literal is not automatically a violation. What matters is the set of event names that can reach PostHog at runtime:

- A reference to a module-level constant, enum, or registry whose value is a literal (`EVENTS.SIGNUP_COMPLETED`, `POSTHOG_EVENTS.BILLING.UPGRADED`) is the **recommended** pattern, not a violation. Follow the reference, confirm it resolves to a literal, and pass it.
- A ternary or switch over string literals resolves to a bounded, statically knowable set. Enumerate the branches and count them.
- A template literal or concatenation that interpolates a runtime value (`` `${action}_clicked` ``, `'event_' + type`) resolves to an unbounded set. This is the case that actually breaks queryability.

Rule:
- Event names in posthog.capture("name", …) must be static strings, not template literals or dynamic variables.
- pass: all capture calls use string literals.
- error: any call uses a template literal or variable as the event name.
- Event names must resolve to a fixed, greppable set of strings.
- pass: every capture name is a string literal, or a constant/enum reference that resolves to one.
- warning: a name is selected from a bounded set of literals (ternary or switch). PostHog still sees a fixed number of definitions, so nothing is unbounded — the real costs are that the literals can't be found by searching the codebase for a name seen in PostHog, and that any dimension folded into the name (origin, plan, variant) belongs in a property where it can drive a breakdown instead.
- error: a name interpolates runtime data, so the set of event definitions is unbounded and each new value creates another definition.

When reporting, state the resolved names or the interpolated expression you found. Never describe a bounded ternary as "unbounded", and don't warn about event-definition or rate limits for a case whose name count is fixed.

Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `capture-event-names-static`, including `file` (path:line of the first violation if any, otherwise of a representative capture call) and `details` (one-line explanation). Return when the call completes. Do not write the audit report.
```
Expand Down Expand Up @@ -77,16 +86,24 @@ You are an audit subagent. Resolve exactly one rule and return: capture-growth-e

Read this skill's bundled `best-practices.md` reference once (typically `.claude/skills/audit/references/best-practices.md`; otherwise discover with `Glob` `**/skills/audit/references/best-practices.md`).

Run **two** Greps in parallel:
- `posthog\.capture\(` — explicit capture calls
Run **three** Greps in parallel:
- `posthog\.capture\(` — browser-SDK capture calls
- `\.capture\(\s*\{` — server-SDK (`posthog-node`) captures, which take a single object (`{ distinctId, event }`) and usually sit behind a wrapper
- `signup|signin|register|checkout|purchase|subscribe|onboard` — likely growth-funnel surfaces

Read each file that contains a hit, once. Cross-reference: do the growth-funnel surfaces actually emit explicit capture calls?
Read each file that contains a hit, once. Cross-reference: do the growth-funnel surfaces emit capture calls, on either runtime?

**Growth events are most often emitted server-side or behind a wrapper, so a browser-only search will wrongly report them missing.** Before concluding any event is absent:

- Follow any in-house analytics wrapper (`analyticsTrack`, `track`, `captureEvent`, an `analytics.*` module) to check whether it forwards to PostHog. A wrapper that fans out to several vendors still counts as instrumentation.
- Look for a central event-name registry — a union type, enum, or constants map of permitted event names. Entries like `user created` or `subscription purchased` are strong evidence the event exists; grep the call sites to confirm it actually fires.
- Signup and purchase in particular tend to live in backend or queue/worker code (billing webhooks, post-signup jobs), not in the UI surface that matched the third grep.
- Note any environment gate on the wrapper (e.g. `if (NODE_ENV === 'production')`). The event exists but won't appear in non-production projects — report that as context, not as a missing event.

Rule:
- Signup, activation/first-key-action, and purchase/subscription should be tracked explicitly. Autocapture isn't enough for funnels.
- pass: at least signup + one activation + (purchase or subscribe) are captured explicitly.
- warning: one or more growth events missing — list which.
- pass: at least signup + one activation + (purchase or subscribe) are captured explicitly, on either the client or the server.
- warning: one or more growth events missing — list which, and state where you looked (browser captures, server captures, wrappers, registry) so the reader can judge the claim.
- Skip (`pass` with details: "no auth/billing paths detected"): no detectable signup/billing surfaces.

Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `capture-growth-events`, including `file` (path:line of the most relevant capture or growth-surface site) and `details` (one-line explanation, listing missing growth events when applicable). Return when the call completes. Do not write the audit report.
Expand Down
31 changes: 30 additions & 1 deletion context/skills/audit/references/5-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ Numbered list, ordered by severity (errors → warnings → suggestions), then b
2. **Why it matters** — one sentence on the data-quality consequence: which downstream artifact (funnels, retention, person count, billing, replays, experiments, etc.) this finding contaminates if left alone, and how. Use the canonical "why it matters" copy below verbatim when the check id matches; otherwise write one sentence rooted in the check's rule.
3. **How to fix** — one short imperative sentence pointing at `file:line` and the concrete change. End with a docs link.

**Keep the claim proportionate to what the subagent actually found.** The canonical copy below describes each check's failure in its general form. When the subagent's `details` say the problem is bounded, already mitigated, or narrower than that general form, describe what was found — do not substitute the generic worst case. Concretely: don't call a fixed set of event names "unbounded", don't claim corrupted person profiles when the code sets `$process_person_profile: false`, and don't assert a limit or quota will be hit unless the finding shows unbounded growth. Inventing severity the evidence doesn't support costs the reader more than it saves, because they have to re-derive the finding themselves before they can act on it.

Format:

```markdown
Expand All @@ -163,6 +165,33 @@ For each `area` from the ledger, in first-seen order:
[Per the investigation standards in `posthog-best-practices/references/investigation-standards.md`, standard 3. ≤4 sentences answering: which code paths were not checked, which runtime assumptions are unproven by static code, what alternative explanations exist for the patterns found, and what to verify in the live PostHog project to confirm the most important findings. When the area produced only `pass` rows, write `_No findings to qualify; the standard checks for this area passed cleanly._` instead.]
```

### Canonical "why it matters" copy

Use the line matching the check id verbatim, subject to the proportionality rule above. For a check id not listed here, write one sentence rooted in that check's rule.

| Check id | Why it matters |
|---|---|
| `sdk-installed` | With no SDK in any dependency manifest, nothing is captured at all — every downstream insight is empty by construction. |
| `sdk-up-to-date` | Minor releases carry bug fixes and capture-reliability improvements, and each release you fall behind widens the gap you'll have to close in a hurry when a security patch lands. |
| `init-correct` | An init in the wrong runtime, or two inits racing in one runtime, means captures are silently dropped or double-counted, so every event volume built on them is wrong. |
| `identify-stable-distinct-id` | A `distinct_id` that resets splits one human across many person records, inflating person counts and breaking any retention or lifecycle analysis that assumes one row per user. |
| `identify-not-late` | Captures and flag evaluations that run before `identify()` are attributed to an anonymous person, so the first steps of every funnel detach from the user who completed them. |
| `cross-runtime-distinct-id` | When client and server disagree on a user's `distinct_id`, that user's activity splits across two person records and no funnel spanning both runtimes can be trusted. |
| `identify-reset-on-logout` | Without a reset, the next user on a shared device inherits the previous user's anonymous ID, and the following `identify()` merges both accounts into a single person profile — corrupting person counts, funnel attribution, and retention cohorts for both. |
| `capture-event-names-static` | An event name assembled at runtime can't be found by searching the codebase for a name seen in PostHog, and a name that interpolates runtime data creates a fresh event definition per value, leaving those events unusable in insights and funnels. |
| `capture-uses-proxy` | Browser captures sent to PostHog's default host are blocked outright for the meaningful share of users running ad or tracking blockers, so the data is missing rather than merely delayed. |
| `capture-growth-events` | Without explicit signup, activation, and purchase events, the conversion funnels those steps define can't be built at all; autocapture can't reliably stitch them across sessions. |

### Canonical area copy

Use the paragraph matching the area name verbatim under that area's heading. For an area not listed here, write one short sentence summarizing what its checks verify.

| Area | Paragraph |
|---|---|
| `Installation` | This area verifies that the PostHog SDK is present in at least one dependency manifest and that its installed version is reasonably current. It also confirms the SDK is initialized correctly — token sourced from environment configuration, called in the right runtime — and that no duplicate init sites race each other within one runtime. |
| `Identification` | This area checks that every `posthog.identify()` call uses a stable, authenticated identifier rather than a session or device id, that identification happens early enough that no captures or flag evaluations fire before the user is known, that client and server agree on one `distinct_id` per user, and that logout and account-switch flows call `posthog.reset()` to prevent cross-user identity merges. |
| `Event Capture` | This area checks that event names passed to `posthog.capture()` resolve to a fixed, greppable set of strings rather than being assembled from runtime data, that browser captures route through a first-party reverse proxy so ad and tracking blockers don't drop them, and that the three growth-funnel events — signup, first activation action, and purchase or subscription — are instrumented explicitly on some runtime rather than left to autocapture. |

After the report is written, emit a line so the wizard can surface the path to the user:

```
Expand Down Expand Up @@ -322,7 +351,7 @@ What `new_value` looks like for each placeholder family:
| `__SUMMARY_PROBLEMATIC__` | A `table` with the Severity / Area / Check / File / Details columns. If there are no problematic items, send a single `paragraph` with the italicised "No issues found" line instead. Either way, fill the placeholder. |
| `__RECOMMENDED_ACTIONS__` | An `orderedList` with one `listItem` per action, in severity order. If there are none, send a `paragraph` with the italicised "Nothing to fix" line. |
| `__FULL_AUDIT_<AREA>_HEADING__` | A level-3 `heading` with the area name (e.g. `Installation`). |
| `__FULL_AUDIT_<AREA>_PARAGRAPH__` | A single `paragraph` with the canonical area framing (see "Canonical area copy" below). |
| `__FULL_AUDIT_<AREA>_PARAGRAPH__` | A single `paragraph` with the canonical area framing (see "Canonical area copy" under Section body templates). |
| `__FULL_AUDIT_<AREA>_TABLE__` | A `table` with header row (Check / Status / File / Details) + one row per check in that area, in ledger order. |
| `__ABOUT_PARAGRAPH__` | A single `paragraph` with the canonical opening sentence about the five-stage chain. |
| `__ABOUT_BULLETS__` | A `bulletList` with the three error/warning/suggestion description bullets. |
Expand Down