Skip to content

feat(telemetry): instrument the install -> boot -> canvas funnel - #1132

Merged
deepme987 merged 4 commits into
mainfrom
deepme987/desktop/funnel-instrumentation
Jun 18, 2026
Merged

feat(telemetry): instrument the install -> boot -> canvas funnel#1132
deepme987 merged 4 commits into
mainfrom
deepme987/desktop/funnel-instrumentation

Conversation

@deepme987

Copy link
Copy Markdown
Collaborator

Summary

Adds the missing desktop-side funnel steps between download and first render, so we can see where users drop instead of only knowing that a launch started. Companion to the cloud-side funnel instrumentation.

Events added

Event When Provider
comfy.desktop.install.phase Per-phase install boundary (start / end / error) PostHog funnel + Datadog mirror (error rows alert)
comfy.desktop.comfyui.boot_phase Launch-progress phase timings PostHog, failure-only (buffered, flushed on boot failure)
comfy.desktop.comfyui.boot_failed Port-wait timeout / early exit / renderer load failure / render-process-gone PostHog + Datadog mirror
comfy.desktop.comfyui.canvas_rendered First dom-ready of a LOCAL install's main frame PostHog
comfy.desktop.first_use.abandoned First-use takeover unmounts with no completion path fired PostHog (consent-gated)
comfy.desktop.cloud.entry_blocked Cloud capacity gate on a gated entry (picker / first_use) PostHog (consent-gated)

Review focus

  • boot_phase is failure-only by design (bootPhaseBuffer.ts). boot_started is ~258k/14d in prod; emitting a per-phase event on every healthy boot would multiply that for no analytical gain (healthy-boot timing is already in instance_started.boot_time_ms). So phase timings are buffered in memory per installation_id and flushed only when the boot fails/times out, as the breakdown that explains where it stalled. Buffers are bounded (one per id, one entry per phase) and terminally cleared (success clears, failure flushes-then-clears, re-attempt resets).
  • Datadog mirror additions (datadogMirroredEvents.ts): only install.phase (filter status:error) and boot_failed ride the mirror; boot_phase is PostHog-only funnel data.
  • canvas_rendered is local-only — the cloud entry has its own cloud.entered path; this fires on the local did-frame-finish-load, deduped per launch, with a separate failed-load leg.
  • cloud.entry_blocked reports the raw capacity flag + tier + decision (no_op = hard-blocked, declined = backed out of the degraded warning, proceeded = entered through it), so a paid user relaxed past a disabled kill-switch reads as status: disabled, tier: paid, decision: proceeded. Skipped on a normal flag.

Follow-up (not in this PR)

  • Folding port_retries into instance_started (P1 refinement) is deferred.
  • Phase 2: routing the embedded local frontend's existing clickstream through the desktop telemetry pipeline (where users click / drop inside the local app).

Companion PRs

Adds the missing desktop-side funnel steps so we can see where users drop
between download and first render, instead of only knowing a launch started.

- comfy.desktop.install.phase — per-phase install boundaries (start / end /
  error) for the standalone installer. PostHog carries the funnel timing;
  the error rows ride the Datadog mirror so a monitor can page when a phase
  hard-fails for a population after a release.
- comfy.desktop.comfyui.boot_phase — launch-progress phase timings, buffered
  in memory per installation and flushed ONLY when the boot fails or times
  out (healthy-boot timing is already covered by instance_started, and
  boot_started alone is ~258k/14d — emitting per-phase on every boot would
  multiply that for no gain). The phases explain WHERE a failed boot stalled.
- comfy.desktop.comfyui.boot_failed — port-wait timeout / early process exit /
  renderer load failure / render-process-gone. Datadog-mirrored; paired with
  the flushed boot_phase breakdown.
- comfy.desktop.comfyui.canvas_rendered — first dom-ready of a LOCAL install's
  main frame (the bottom of the install->canvas funnel), with
  server_ready_to_canvas_ms. Deduped per launch; the failed-load leg is
  recorded separately.
- comfy.desktop.first_use.abandoned — the first-use takeover unmounting
  without any completion path firing (the chooser-drop signal; pairs with
  first_use.completed to give onboarding its denominator).
- comfy.desktop.cloud.entry_blocked — the cloud capacity gate on every gated
  entry (picker / first_use), with raw flag + tier + decision, so we can see
  how many cloud entries the kill-switch shed vs. warned vs. let through.

boot_phase / install.phase buffers are bounded and terminally cleared.
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR instruments multiple telemetry funnels across the desktop app: boot-phase timings buffered and only emitted on failure, canvas-rendered dedup tracking with success/failure signals, install-phase boundary callbacks wired through the installer and standalone install flow, session retry metrics propagated from main to renderer via IPC, and cloud capacity entry gating enriched with a source context for attribution. A first-use abandonment event is also added.

Changes

Main-process boot, install, and canvas telemetry

Layer / File(s) Summary
Boot phase buffer module and tests
src/main/lib/bootPhaseBuffer.ts, src/main/lib/bootPhaseBuffer.test.ts
New bootPhaseBuffer module keyed by installationId records phase timings during boot, emits comfy.desktop.comfyui.boot_phase events per phase only on flushBootPhasesOnFailure, and silently discards them on clearBootPhases (success path). Tests cover all semantics including dedup, timing, isolation, and flush.
Launch progress tracker onPhaseEnter wiring
src/main/lib/launchProgress.ts, src/main/lib/ipc/sessionActions/launch.ts
createLaunchProgressTracker gains a swallowed-exception onPhaseEnter callback. launch.ts wires it to recordBootPhase, calls startBootPhases per attempt, and on terminal failure flushes phase data and emits comfy.desktop.comfyui.boot_failed; on success/cancel it calls clearBootPhases. Successful sessions now carry portRetries/rebootRetries into _addSession.
Installer phase boundary telemetry
src/main/lib/installer.ts, src/main/sources/standalone/install.ts
installer.ts exports InstallPhaseName/InstallPhaseStatus types, extends InstallerContext with an optional onPhase hook, and wraps download/extract stages with a withInstallPhase helper. standalone/install.ts adds emitInstallPhase/withPostInstallPhase emitting comfy.desktop.install.phase, instruments env_create, package_copy, and torch_deps_sync phases, and maps the installer's onPhase callback to the emitter.
Canvas rendered funnel telemetry
src/main/lib/canvasEntry.ts, src/main/lib/canvasEntry.test.ts, src/main/lib/ipc/shared.ts, src/main/host/attach.ts, src/main/index.ts
New canvasEntry.ts emits comfy.desktop.comfyui.canvas_rendered once per launch per install (dedup bypassed for loadFailed), computing server_ready_to_canvas_ms from a new getSessionStartedAt helper in shared.ts. attach.ts fires the event on dom-ready and on frame-load failure. index.ts resets the dedup guard at onLaunch. Tests cover dedup, reset, loadFailed semantics, and timing.
Session retry metrics in IPC and renderer
src/types/ipc.ts, src/main/lib/ipc/shared.ts, src/renderer/src/lib/rendererBootstrap.ts, src/shared/datadogMirroredEvents.ts
RunningInstance gains optional bootTimeMs/portRetries/rebootRetries fields. _addSession includes retry counts in the instance-started broadcast. rendererBootstrap.ts normalizes the payload and adds port_retries/reboot_retries to comfy.desktop.session.instance_started. The Datadog allow-list gains comfy.desktop.install.phase and comfy.desktop.comfyui.boot_failed.

Cloud capacity entry source tagging and first-use abandonment

Layer / File(s) Summary
CloudEntrySource type, confirmEntry signature, and emitGate
src/renderer/src/composables/useCloudCapacity.ts
Adds CloudEntrySource union type ('picker' | 'first_use'), updates confirmEntry to require a source argument, and introduces emitGate emitting cloud.entry_blocked with status/tier/decision/source in the disabled (no_op) and degraded (proceeded/declined) branches.
Call sites and first-use abandonment event
src/renderer/src/views/ChooserView.vue, src/renderer/src/comfyTitlePopup/InstancePickerView.vue, src/renderer/src/views/FirstUseTakeover.vue
ChooserView and InstancePickerView pass 'picker' to confirmEntry. FirstUseTakeover passes 'first_use' and emits comfy.desktop.first_use.abandoned on unmount when incomplete, reporting step, ToS-acceptance reason, elapsed time, and had_legacy.
useCloudCapacity confirmEntry tests
src/renderer/src/composables/useCloudCapacity.test.ts
Vitest suite re-imports the composable per test to avoid singleton leakage and asserts return values, dialog invocation, and full telemetry payload shapes — including decision values — across all status/tier combinations.

Sequence Diagrams

sequenceDiagram
  participant launch.ts
  participant launchProgress.ts
  participant bootPhaseBuffer
  participant telemetry
  participant _addSession

  launch.ts->>bootPhaseBuffer: startBootPhases(installationId, variant)
  launch.ts->>launchProgress.ts: createLaunchProgressTracker({ onPhaseEnter })
  loop each boot phase entered
    launchProgress.ts->>launch.ts: onPhaseEnter(phase)
    launch.ts->>bootPhaseBuffer: recordBootPhase(installationId, phase)
  end
  alt terminal boot failure
    launch.ts->>bootPhaseBuffer: flushBootPhasesOnFailure(installationId)
    bootPhaseBuffer->>telemetry: emit boot_phase ×N
    launch.ts->>telemetry: emit boot_failed(failedPhase, portRetries, rebootRetries)
  else boot succeeded
    launch.ts->>bootPhaseBuffer: clearBootPhases(installationId)
    launch.ts->>_addSession: _addSession(id, info, bootTimeMs, { portRetries, rebootRetries })
  end
Loading
sequenceDiagram
  participant attach.ts
  participant canvasEntry.ts
  participant ipc/shared.ts
  participant telemetry

  attach.ts->>canvasEntry.ts: noteCanvasRendered(installationId)
  canvasEntry.ts->>ipc/shared.ts: getSessionStartedAt(installationId)
  ipc/shared.ts-->>canvasEntry.ts: startedAt (ms epoch)
  canvasEntry.ts->>telemetry: emit canvas_rendered(server_ready_to_canvas_ms, load_failed=false)
  Note over canvasEntry.ts: dedup guard set — subsequent success calls skipped
  attach.ts->>canvasEntry.ts: noteCanvasRendered(id, { loadFailed: true })
  canvasEntry.ts->>telemetry: emit canvas_rendered(load_failed=true)
  Note over canvasEntry.ts: loadFailed bypasses and does not consume dedup slot
Loading
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch deepme987/desktop/funnel-instrumentation
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch deepme987/desktop/funnel-instrumentation

Comment @coderabbitai help to get the list of available commands and usage tips.

…pacity gate

- bootPhaseBuffer: failure-only flush, first-write-wins per phase, bounded
  lifecycle (start resets, success clears, flush emits one boot_phase per
  buffered phase + returns last phase + clears).
- canvasEntry: first-render dedup per installation, failed-load leg bypasses
  the dedup, server_ready_to_canvas_ms from the session anchor.
- useCloudCapacity.confirmEntry: cloud.entry_blocked decisions
  (no_op / declined / proceeded), normal flag emits nothing, paid-user
  relaxation past a disabled kill-switch.
@deepme987
deepme987 marked this pull request as ready for review June 18, 2026 04:35
@deepme987
deepme987 enabled auto-merge (squash) June 18, 2026 04:36

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/lib/ipc/sessionActions/launch.ts (1)

132-147: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Per-attempt boot-phase buffering is reset, but phase-entry state is not, so retry failures can emit incomplete or wrong phase telemetry.

Because launchTracker is reused and onPhaseEnter only fires on first entry per phase index, later retries can flush sparse/empty boot_phase data and a misleading failed_phase—telemetry should ride shotgun, not grab the wheel.

Also applies to: 717-721, 797-803

🤖 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 `@src/main/lib/ipc/sessionActions/launch.ts` around lines 132 - 147, The
`launchTracker` is reused across retry attempts but its phase-entry tracking
state is not reset, causing `onPhaseEnter` callbacks to not fire on subsequent
retries and resulting in incomplete or incorrect boot phase telemetry. In the
`armLaunchTracker` function, remove the early return that reuses the existing
`launchTracker` so that a fresh tracker is created for each launch attempt,
ensuring `onPhaseEnter` fires properly for all retry attempts. Apply the same
fix pattern to the other retry locations mentioned at lines 717-721 and 797-803
to ensure consistent boot phase telemetry across all retry scenarios.
🤖 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 `@src/main/lib/canvasEntry.ts`:
- Line 34: The calculation of serverReadyToCanvasMs can produce negative values
when the system clock moves backward, which corrupts telemetry data. After
calculating the time difference in the line where serverReadyToCanvasMs is
assigned, apply a Math.max operation to clamp the value to a minimum of 0,
ensuring that any negative values are converted to 0 before the metric is
emitted.

In `@src/main/lib/installer.ts`:
- Around line 51-59: The onPhase callbacks in the withInstallPhase function can
throw exceptions that break the installation process, but they are meant to be
side-channel-only and should never affect the actual install flow. Wrap each of
the three onPhase invocations (the one at the start, the one on success with
durationMs, and the one on error with durationMs and error) in their own
try/catch blocks. In each catch block, silently handle the error without
rethrowing it so that callback failures never abort the install or mask the
actual install errors.

In `@src/main/sources/standalone/install.ts`:
- Around line 42-61: The emitInstallPhase function lacks error handling around
telemetry operations, which can cause the entire installation process to fail if
telemetry classification or emission fails. Wrap the telemetry logic (the
mainTelemetry.bucketError call and mainTelemetry.emit call) in a try/catch block
to ensure telemetry failures are caught and handled gracefully without
interrupting the install execution. The catch block should handle the error
silently or with minimal logging to prevent side-channel metrics from derailing
the installation process.

In `@src/renderer/src/composables/useCloudCapacity.test.ts`:
- Around line 44-45: The type annotation for the `composable` variable on line
44 is incorrectly resolving to the composable function itself rather than its
return value. Change the type of `composable` from `Awaited<ReturnType<typeof
importComposable>>` to properly represent the return type of calling the
useCloudCapacity function (the composable's actual return value, not the
function reference). This ensures the declared type matches what is actually
assigned on line 61 when composable is set to `ReturnType<typeof
useCloudCapacity>`.

---

Outside diff comments:
In `@src/main/lib/ipc/sessionActions/launch.ts`:
- Around line 132-147: The `launchTracker` is reused across retry attempts but
its phase-entry tracking state is not reset, causing `onPhaseEnter` callbacks to
not fire on subsequent retries and resulting in incomplete or incorrect boot
phase telemetry. In the `armLaunchTracker` function, remove the early return
that reuses the existing `launchTracker` so that a fresh tracker is created for
each launch attempt, ensuring `onPhaseEnter` fires properly for all retry
attempts. Apply the same fix pattern to the other retry locations mentioned at
lines 717-721 and 797-803 to ensure consistent boot phase telemetry across all
retry scenarios.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b68e4114-a061-48d7-9e10-d4fb1c104894

📥 Commits

Reviewing files that changed from the base of the PR and between 93ec36d and 7d455dd.

📒 Files selected for processing (19)
  • src/main/host/attach.ts
  • src/main/index.ts
  • src/main/lib/bootPhaseBuffer.test.ts
  • src/main/lib/bootPhaseBuffer.ts
  • src/main/lib/canvasEntry.test.ts
  • src/main/lib/canvasEntry.ts
  • src/main/lib/installer.ts
  • src/main/lib/ipc/sessionActions/launch.ts
  • src/main/lib/ipc/shared.ts
  • src/main/lib/launchProgress.ts
  • src/main/sources/standalone/install.ts
  • src/renderer/src/comfyTitlePopup/InstancePickerView.vue
  • src/renderer/src/composables/useCloudCapacity.test.ts
  • src/renderer/src/composables/useCloudCapacity.ts
  • src/renderer/src/lib/rendererBootstrap.ts
  • src/renderer/src/views/ChooserView.vue
  • src/renderer/src/views/FirstUseTakeover.vue
  • src/shared/datadogMirroredEvents.ts
  • src/types/ipc.ts

Comment thread src/main/lib/canvasEntry.ts
Comment thread src/main/lib/installer.ts
Comment thread src/main/sources/standalone/install.ts
Comment thread src/renderer/src/composables/useCloudCapacity.test.ts
@MaanilVerma MaanilVerma self-assigned this Jun 18, 2026
@MaanilVerma
MaanilVerma self-requested a review June 18, 2026 05:02
@deepme987
deepme987 merged commit c684b68 into main Jun 18, 2026
12 checks passed
@deepme987
deepme987 deleted the deepme987/desktop/funnel-instrumentation branch June 18, 2026 05:02
deepme987 added a commit that referenced this pull request Jun 18, 2026
Follow-ups to the merged install -> boot -> canvas instrumentation (#1132);
all side-channel-hardening, no behavior change to the funnel events:

- canvasEntry: clamp server_ready_to_canvas_ms to >= 0 so a backward clock
  step can't emit a negative, funnel-polluting duration.
- installer.withInstallPhase: isolate every onPhase tap in try/catch so a
  throwing telemetry callback can never abort or mask an install.
- install.emitInstallPhase: guard classification/emission so side-channel
  metrics fail quietly (this is also the installer's onPhase tap).
- useCloudCapacity.test: fix the loadComposable return type — it resolved to
  the composable function, not its call result.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants