feat: PostHog tracking for Capgo Builder onboarding + build lifecycle#2287
Open
WcaleNieWolny wants to merge 14 commits into
Open
feat: PostHog tracking for Capgo Builder onboarding + build lifecycle#2287WcaleNieWolny wants to merge 14 commits into
WcaleNieWolny wants to merge 14 commits into
Conversation
Adds the design doc covering two event families: - Builder Onboarding Step (per-wizard-step, fired CLI -> backend -> sendEventToTracking) with closed-enum error categories - Build lifecycle (Requested / Started / Succeeded / Failed / Timed Out) fired entirely server-side from the existing request handler and reconciliation cron No changes to the capgo_builder repo; build_started is derived from the existing builder polling.
The existing /private/events handler already implements auth, org resolution, app_id permission check, and sendEventToTracking with org grouping. Adding a second endpoint would duplicate ~80 lines of working code. CLI posts directly with the new event name.
10 bite-sized TDD tasks covering iOS + Android error category mappers, the CLI telemetry helper, useEffect wiring in both wizards, pure build-transition / failure-category helpers, Build Requested emission in request.ts, transition events from cron_reconcile_build_status, full verification, and PR creation. Reuses /private/events; no new backend endpoint. capgo_builder repo is not touched.
The wizards previously stored only err.message in React state, then reconstructed `new Error(message)` to pass to the category mapper — losing the .status / .phase / instanceof discriminators the mapper relies on. Capture the mapped category at handleError time via a ref and pass it directly to the telemetry helper, bypassing the mapper for wizard-emitted events. Tests, the helper interface, and both wizards are updated.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR adds end-to-end PostHog telemetry for Capgo Builder: onboarding step events (iOS/Android) with error-category tagging, a telemetry helper with env-opt-out, backend build lifecycle event emission (request & cron transitions), mapping helpers for error/failure categories, unit tests, and design/plan docs. ChangesBuilder Telemetry Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Merging this PR will not alter performance
Comparing 
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
supabase/functions/_backend/triggers/cron_reconcile_build_status.ts (1)
214-225:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTransition telemetry is race-prone without compare-and-set on status.
previousStatusis read from a stale snapshot, but Line 216 update only matchesid. Concurrent reconciliations can both update and both emit the same transition event. Add a status guard in the update (eq('status', previousStatus)) and emit tracking only when that guarded update actually applies.Also applies to: 252-291
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/functions/_backend/triggers/cron_reconcile_build_status.ts` around lines 214 - 225, The current update to build_requests uses only .eq('id', build.id) and reads previousStatus from a snapshot, so concurrent reconciliations can both update and both emit transition telemetry; change the update call in the reconciliation logic to include a status guard by adding .eq('status', previousStatus) to the Supabase query (the call on supabase.from('build_requests').update({...}).eq('id', build.id)), then only emit the transition/telemetry when the guarded update actually applied (i.e., when the update returned a successful result/affected row(s) rather than an error or zero rows). Apply the same change to the second reconciliation block covering the code around the other update/emit (the block referenced in the review for lines 252-291).
🧹 Nitpick comments (2)
docs/superpowers/specs/2026-05-18-capgo-builder-posthog-tracking-design.md (1)
110-136: 💤 Low valueAdd language identifier to code fence.
The fenced code block at line 110 lacks a language specification, which triggers a markdown linting warning (MD040). Since this is an ASCII architecture diagram, specify
textorplaintextas the language identifier.📝 Proposed fix
-``` +```text ONBOARDING:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-05-18-capgo-builder-posthog-tracking-design.md` around lines 110 - 136, The fenced ASCII diagram starting with "ONBOARDING:" should include a language identifier to satisfy MD040; update the opening code fence that wraps the diagram (the block showing ONBOARDING, BUILDS, sendEventToTracking, trackPosthogEvent, etc.) to use ```text (or ```plaintext) instead of just ```, so the block becomes a text/plaintext code fence.tests/builder-onboarding-telemetry.unit.test.ts (1)
24-143: ⚡ Quick winUse
it.concurrentfor these test cases to match repo test policy.These tests currently use
it(...); the repository test guideline fortests/**/*.test.tsrequiresit.concurrent(...)for parallel execution.As per coding guidelines, "Design all tests for parallel execution across files; use it.concurrent() instead of it() to run tests in parallel within the same file for faster CI/CD".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/builder-onboarding-telemetry.unit.test.ts` around lines 24 - 143, Update each test declaration in this file to run concurrently by replacing it(...) with it.concurrent(...); specifically modify the tests with descriptions like "builds the expected payload and calls sendEvent once", "includes error_category only when an error is provided", "uses the Android mapper when platform is android", "skips when CAPGO_DISABLE_TELEMETRY is set", "skips when CAPGO_DISABLE_POSTHOG is set", "swallows errors thrown by sendEvent", "does not include duration_ms when undefined", and "uses pre-computed errorCategory when provided (skipping the mapper)" so they call it.concurrent and otherwise keep the same callbacks that call trackBuilderOnboardingStep and assert on sendEventMock.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/src/build/onboarding/android/ui/app.tsx`:
- Around line 68-70: Reorder the three imports in app.tsx to satisfy the
perfectionist/sort-imports rule: locate the imports for
mapAndroidOnboardingError, trackBuilderOnboardingStep, and
ANDROID_STEP_PROGRESS/getAndroidPhaseLabel and reorder them to match the
repository's configured import groups/sort order (or run the linter autofix) so
the import sequence and grouping match perfectionist's expectations.
In `@docs/superpowers/plans/2026-05-18-capgo-builder-posthog-tracking.md`:
- Line 572: The current code wraps the wizard state error string as new
Error(error) which loses properties inspected by mapIosOnboardingError and
mapAndroidOnboardingError; instead pass the original error object (from wizard
state) or precompute and pass the mapped category to the telemetry helper.
Locate where error: step === 'error' && error ? new Error(error) : undefined is
set and change it to forward the original error object (e.g., errorObject) or
call the mapper (mapIosOnboardingError / mapAndroidOnboardingError) at that
point and pass the resulting category into the telemetry helper so the telemetry
receives the preserved onboarding error_category.
In `@docs/superpowers/specs/2026-05-18-capgo-builder-posthog-tracking-design.md`:
- Line 206: Update the stale endpoint reference: find the phrase mentioning "the
new /private/track_onboarding endpoint" in the spec and either change it to
reference the reused "/private/events" endpoint or remove the endpoint-specific
wording (e.g., replace with "No per-org rate limit on onboarding events");
ensure the surrounding text remains consistent with the architectural decision
to reuse /private/events.
- Line 23: Update the sentence that reads "Sent from the CLI through a new
backend endpoint" to clarify that the CLI uses the existing endpoint by
referencing "/private/events" (e.g., "Sent from the CLI through the existing
/private/events endpoint") so it matches the architectural decision described in
the section around the CLI wizard step transition and the reuse of the
/private/events endpoint.
In `@supabase/functions/_backend/public/build/request.ts`:
- Around line 317-329: The awaited sendEventToTracking call can throw and should
be best-effort: wrap the call to sendEventToTracking(c, { ... }) in a try/catch
so any rejection is caught, log the error (e.g., console.error or the
request/logger available on c) with context "Build Requested telemetry failed"
and do not rethrow or change the API response; keep the original behavior for
successful paths. Ensure you reference the existing sendEventToTracking
invocation and its context parameter c when applying the try/catch so telemetry
failures do not convert a successful build request into an API error.
In `@supabase/functions/_backend/utils/build_tracking.ts`:
- Around line 19-24: The current early-return checks evaluate equality before
honoring a timeout override, causing input.timeoutApplied true with
input.previous === input.next to return null; update the conditional order or
add a branch so that when input.timeoutApplied is true the function returns
'timed_out' regardless of equality — e.g., check input.timeoutApplied (or add a
specific conditional for input.timeoutApplied && input.previous === input.next)
before/above the input.previous === input.next check in the build/tracking logic
so callers passing raw next statuses get the 'timed_out' transition.
In `@tests/builder-onboarding-telemetry.unit.test.ts`:
- Line 9: Move the import of trackBuilderOnboardingStep so it appears at the top
of tests/builder-onboarding-telemetry.unit.test.ts before any runtime
statements; the lint error is caused by the import occurring after executable
code, so locate the current import of trackBuilderOnboardingStep and place it
with the other top-level imports to satisfy the import/first rule.
---
Outside diff comments:
In `@supabase/functions/_backend/triggers/cron_reconcile_build_status.ts`:
- Around line 214-225: The current update to build_requests uses only .eq('id',
build.id) and reads previousStatus from a snapshot, so concurrent
reconciliations can both update and both emit transition telemetry; change the
update call in the reconciliation logic to include a status guard by adding
.eq('status', previousStatus) to the Supabase query (the call on
supabase.from('build_requests').update({...}).eq('id', build.id)), then only
emit the transition/telemetry when the guarded update actually applied (i.e.,
when the update returned a successful result/affected row(s) rather than an
error or zero rows). Apply the same change to the second reconciliation block
covering the code around the other update/emit (the block referenced in the
review for lines 252-291).
---
Nitpick comments:
In `@docs/superpowers/specs/2026-05-18-capgo-builder-posthog-tracking-design.md`:
- Around line 110-136: The fenced ASCII diagram starting with "ONBOARDING:"
should include a language identifier to satisfy MD040; update the opening code
fence that wraps the diagram (the block showing ONBOARDING, BUILDS,
sendEventToTracking, trackPosthogEvent, etc.) to use ```text (or ```plaintext)
instead of just ```, so the block becomes a text/plaintext code fence.
In `@tests/builder-onboarding-telemetry.unit.test.ts`:
- Around line 24-143: Update each test declaration in this file to run
concurrently by replacing it(...) with it.concurrent(...); specifically modify
the tests with descriptions like "builds the expected payload and calls
sendEvent once", "includes error_category only when an error is provided", "uses
the Android mapper when platform is android", "skips when
CAPGO_DISABLE_TELEMETRY is set", "skips when CAPGO_DISABLE_POSTHOG is set",
"swallows errors thrown by sendEvent", "does not include duration_ms when
undefined", and "uses pre-computed errorCategory when provided (skipping the
mapper)" so they call it.concurrent and otherwise keep the same callbacks that
call trackBuilderOnboardingStep and assert on sendEventMock.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 86155c78-e235-4024-b26b-5c9006cfabab
📒 Files selected for processing (14)
cli/src/build/onboarding/android/types.tscli/src/build/onboarding/android/ui/app.tsxcli/src/build/onboarding/error-categories.tscli/src/build/onboarding/telemetry.tscli/src/build/onboarding/types.tscli/src/build/onboarding/ui/app.tsxdocs/superpowers/plans/2026-05-18-capgo-builder-posthog-tracking.mddocs/superpowers/specs/2026-05-18-capgo-builder-posthog-tracking-design.mdsupabase/functions/_backend/public/build/request.tssupabase/functions/_backend/triggers/cron_reconcile_build_status.tssupabase/functions/_backend/utils/build_tracking.tstests/build-tracking-helpers.unit.test.tstests/builder-onboarding-telemetry.unit.test.tstests/onboarding-error-categories.unit.test.ts
Comment on lines
+317
to
+329
| await sendEventToTracking(c, { | ||
| event: 'Build Requested', | ||
| channel: 'build-lifecycle', | ||
| icon: '🛠️', | ||
| notify: false, | ||
| user_id: org_id, | ||
| groups: { organization: org_id }, | ||
| tags: { | ||
| app_id, | ||
| platform, | ||
| build_mode, | ||
| }, | ||
| }) |
Contributor
There was a problem hiding this comment.
Don’t let telemetry failure fail a successful build request.
This awaited tracking call can reject and turn a successful persisted build request into an API error. Please make this best-effort (log and continue).
Proposed fix
- await sendEventToTracking(c, {
- event: 'Build Requested',
- channel: 'build-lifecycle',
- icon: '🛠️',
- notify: false,
- user_id: org_id,
- groups: { organization: org_id },
- tags: {
- app_id,
- platform,
- build_mode,
- },
- })
+ try {
+ await sendEventToTracking(c, {
+ event: 'Build Requested',
+ channel: 'build-lifecycle',
+ icon: '🛠️',
+ notify: false,
+ user_id: org_id,
+ groups: { organization: org_id },
+ tags: {
+ app_id,
+ platform,
+ build_mode,
+ },
+ })
+ }
+ catch (error) {
+ cloudlogErr({
+ requestId: c.get('requestId'),
+ message: 'Failed to emit Build Requested tracking event',
+ app_id,
+ org_id,
+ error: (error as Error)?.message ?? String(error),
+ })
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/functions/_backend/public/build/request.ts` around lines 317 - 329,
The awaited sendEventToTracking call can throw and should be best-effort: wrap
the call to sendEventToTracking(c, { ... }) in a try/catch so any rejection is
caught, log the error (e.g., console.error or the request/logger available on c)
with context "Build Requested telemetry failed" and do not rethrow or change the
API response; keep the original behavior for successful paths. Ensure you
reference the existing sendEventToTracking invocation and its context parameter
c when applying the try/catch so telemetry failures do not convert a successful
build request into an API error.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c595d2c6d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+252
to
+256
| const transition = classifyBuildTransition({ | ||
| previous: previousStatus, | ||
| next: effectiveStatus, | ||
| timeoutApplied, | ||
| }) |
There was a problem hiding this comment.
This new transition emitter only runs inside cron_reconcile_build_status, but build status is also advanced in request-time handlers (public/build/start.ts sets status to running and public/build/status.ts updates terminal states + updated_at). Because this cron processes only stale non-terminal rows, builds that are actively polled or started through those endpoints can skip the transition window entirely, so Build Started/terminal lifecycle events are never sent. That makes PostHog lifecycle funnels incomplete for normal build flows; emit the same tracking when those other status writes occur (or centralize status-transition tracking in a shared write path).
Useful? React with 👍 / 👎.
- android/ui/app.tsx: reorder telemetry imports next to other ../../ depth imports to satisfy perfectionist/sort-imports - builder-onboarding-telemetry.unit.test.ts: move trackBuilderOnboardingStep import above the vi.mock setup to satisfy import/first (vi.mock is hoisted by Vitest) - build_tracking.ts: timeoutApplied now overrides the previous===next no-change check so a stale-snapshot caller passing equal statuses with timeoutApplied=true still emits 'timed_out'. Adds matching test case. - docs/spec: drop stale references to the never-built /private/track_onboarding endpoint; clarify the CLI uses the existing /private/events route. Tag the ASCII architecture diagram with the 'text' language for MD040. - docs/plan: replace the stale `new Error(error)` snippet in both wizard wirings with the shipped errorCategoryRef + errorCategory: ... pattern, and add a one-line note pointing to the fix. Skipped (with reasons): - request.ts try/catch around sendEventToTracking: redundant. sendEventToTracking already swallows per-provider errors via runTrackedCall and returns Promise.resolve(null) via backgroundTask in production. Adding a wrap would diverge from every other call site (on_app_create.ts, stripe_event.ts, etc.) that follows the same pattern. - cron_reconcile_build_status.ts optimistic-concurrency guard on the build_requests update: pre-existing systemic concern. None of the cron's other state mutations (recordBuildTime, status updates) use such a guard either. Adding it just for telemetry would be partial and inconsistent; out of scope. - Converting it() to it.concurrent() in builder-onboarding-telemetry.unit.test.ts: the tests mutate shared process.env via beforeEach/afterEach. Concurrent execution would race on the env vars and produce flakes.
|
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
1 participant
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.
Summary
Adds PostHog tracking (via the existing
sendEventToTrackingdual-writer) for two event families in the Capgo Builder flow.1. Per-step CLI onboarding tracking
Builder Onboarding Stepfired per wizard step transition in both iOS (cli/src/build/onboarding/ui/app.tsx) and Android (cli/src/build/onboarding/android/ui/app.tsx) onboarding wizards./private/eventsendpoint via the existingsendEvent()helper — no new backend endpoint.step,platform,app_id, optionalduration_ms, optionalerror_category.error_categoryis a closed enum (iOS:apple_api_unauthorized,apple_api_rate_limited,cert_limit_reached,profile_creation_failed,p8_invalid,unknown; Android:keystore_invalid,google_oauth_failed,play_account_id_invalid,unknown). Raw error messages never leave the CLI.cli/src/build/onboarding/telemetry.tshonorsCAPGO_DISABLE_TELEMETRY/CAPGO_DISABLE_POSTHOG.2. Server-side build lifecycle tracking
Build Requestedfires frompublic/build/request.tsafter thebuild_requestsrow is inserted.Build Started,Build Succeeded,Build Failed,Build Timed Outfire fromtriggers/cron_reconcile_build_status.tson status transitions.classifyBuildTransitioninutils/build_tracking.ts. Idempotency is enforced by skipping when the previous status is already terminal — reuses the canonicalTERMINAL_BUILD_STATUSESset fromutils/build_timeout.ts.failure_categoryis a closed enum (timeout,builder_error,validation_error,unknown). Raw error text never reaches PostHog.Out of scope: the
capgo_builderrepo is not modified.Build Startedis derived from the existing reconciliation cron diff, not a new builder push.What's not changed
/private/events.CAPGO_DISABLE_TELEMETRY/CAPGO_DISABLE_POSTHOG).waitUntil); no change to its completion model.Test plan
bun run cli:checkpassestests/tracking.unit.test.ts,tests/posthog.unit.test.ts,tests/builder-payload.unit.test.ts,tests/build-timeout.unit.test.tsnpx @capgo/cli build init --platform=iostwo steps in staging; confirmBuilder Onboarding Stepevents arrive in PostHog with the expectedstep,platform,app_id, and an orggroupsassociationnpx @capgo/cli build requestagainst staging; confirmBuild Requested→Build Started→Build Succeeded/Build Failedarrive in PostHog withduration_secondson terminal events andfailure_categoryon failureserror_categoryorfailure_categoryvalue is anything other than the closed-enum members, and no raw error strings or file paths appear in any propertyDocumentation
Spec:
docs/superpowers/specs/2026-05-18-capgo-builder-posthog-tracking-design.mdPlan:
docs/superpowers/plans/2026-05-18-capgo-builder-posthog-tracking.mdKnown limitations called out by reviewers but kept as-is
Build Requestedevent becausebuild_requestshas no idempotency key. Pre-existing behavior; the spec acknowledges this.sendEventToTracking's default background-task mode (the established convention across every other call site). Switching to foreground would serialize the cron loop and is left for a separate discussion.Summary by CodeRabbit
New Features
Documentation
Tests