feat(cli): align primitive funnel measurement - #3052
Open
miguel-heygen wants to merge 3 commits into
Open
Conversation
Terminal funnel events took a durable single-use claim before they were sent, but delivery was the fire-and-forget exit flush: a detached child with no retry and no way to report failure. An unacknowledged send left the claim spent on an event PostHog never received, so that step could never be emitted again. Observed in a real install, where primitive_preview_succeeded was claimed and never arrived. The terminal path now awaits delivery and hands the claim back when the batch is not acknowledged. That trades a permanent loss for a possible duplicate, which is the cheaper failure: every funnel event carries a stable event_id, so duplicates collapse downstream while a loss is unrecoverable. Claims are also no longer spent while telemetry is opted out. Separately, funnel order is now carried by an explicit funnel_step rather than inferred from timestamps. Auth completion and install start are emitted back to back inside one command and land in the same millisecond, so ordering by time resolved them arbitrarily and could report an install beginning before the auth that authorized it.
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.
What
Makes the primitive install funnel introduced in #3046 actually measurable: one canonical ordered event vocabulary, bounded event properties, anonymous result delivery across every catalog output mode, funnel continuity across the OAuth process boundary, and process-atomic terminal claims for preview and render.
Instrumentation correctness only. No product behavior changes.
Why
The first slice emitted events, but it did not guarantee they were the same events, in the same order, carrying the same funnel id, once the flow crossed a process boundary or ran concurrently. That leaves three failure modes that make any downstream read of the journey untrustworthy:
NaNreach the transport.Two further failures were found by running the built CLI against the real ingest endpoint rather than against mocks, and are fixed here:
primitive_preview_succeededand it never arrived. Because the claim is durable and the delivery was not, that step could never be emitted again.primitive_auth_completedandprimitive_install_startedare emitted back to back inside one command and land in the same millisecond. Two identical warehouse queries returned them in opposite orders, so a funnel keyed on the sequence would intermittently see the install beginning before the auth that authorized it.How
PRIMITIVE_FUNNEL_SIDE_EFFECTSintelemetry/primitive-funnel.tsmaps each lifecycle step to its event name, its stable event-id suffix, and its position in the canonical order. Every emitter goes through it, so a rename cannot drift between call sites.step, emitted asfunnel_step. Consumers order by it instead of by a millisecond that ties. Failure steps share the number of the step they terminate; shipped numbers are never reused.O_EXCL(wx), named by the sha256 of the event id, under.hyperframes/primitive-funnel-claims/. First writer wins, everyone else is a no-op. Repeated 32-process preview and render races produced exactly one claimant each time.flush()now reports whether PostHog acknowledged the batch, the terminal path awaits it, andreleasePrimitiveFunnelEventhands the claim back when it did not. This is deliberately at-least-once: every event carries a stableevent_id, so a duplicate collapses downstream, while a loss is unrecoverable. Claims are also no longer spent while telemetry is opted out.result_countis bounded to 0 to 1000,result_rankto 1 to 1000,duration_msto 0 to 86,400,000. Non-finite input collapses to the nearest bound instead of leakingNaN.primitive-funnel-state.tspersists the context through temp-file plus rename with0600/0700modes, and re-validates every field on read. A malformed state file degrades to "no funnel" rather than throwing, so telemetry can never break the command it instruments.auth_state: anonymous.addcarries the same funnel id through OAuth, install, preview, and render.Test plan
Full CLI suite on Linux: 187 files, 2,508 passed, 2 skipped, 0 failed.
Focused telemetry, funnel, auth, resume, and catalog suites: 19 files, 237 passed.
Concurrency: repeated 32-process preview and render races, exactly one terminal claimant per race.
New coverage: claim released on unacknowledged delivery and re-emitted by a later command, claim retained on acknowledgement, no claim spent while opted out, and a
funnel_stepordering assertion over the delivered batches.End-to-end against real ingest, three separate installs of the built CLI: anonymous discovery with no session, add with caller-supplied messages, background preview, and a real 8-frame 1920x1080 MP4. Confirmed in the warehouse that the terminal preview event now arrives, and that ordering by
funnel_stepputs auth completion before install start even though both events still share one millisecond.Typecheck,
oxfmt --check,oxlint: clean.Unit tests added/updated
Manual testing performed (three live end-to-end installs of the built CLI, verified against real ingested events)
Documentation updated (if applicable) - no user-facing surface changed
Known limitation
A direct
hyperframes addcannot produce steps 1 and 2. Funnel state is only persisted at install, so a precedingcatalogsearch carries its own funnel id and does not stitch to the add that follows. Only the interactive catalog path produces the full ordered journey. That is a scoping question about what the funnel should mean, not a defect, so it is deliberately unchanged here.Stack
Based on
feat/thread-message-stack(#3046); merge that first. This PR proves instrumentation correctness only. It does not claim production adoption, which stays gated on merging and releasing the stack and then a separately authorized read of real journey data.