diff --git a/docs/assets-performance-telemetry.md b/docs/assets-performance-telemetry.md new file mode 100644 index 000000000..cd92555f7 --- /dev/null +++ b/docs/assets-performance-telemetry.md @@ -0,0 +1,97 @@ +# Assets performance telemetry contract + +Desktop compares boot duration and outcomes for launches with and without the Assets argument. +This is an observational comparison, not a controlled experiment or a measurement of causal effect. +It uses the existing ComfyUI boot lifecycle without adding Core instrumentation or asset metadata. + +## Cohort contract + +Each `comfy.desktop.comfyui.boot_started`, `boot_completed`, and `boot_failed` event carries: + +- `assets_enabled`: whether the final launch arguments contain `--enable-assets`, including + manual/source arguments. +- `core_beta_flags`: managed Core beta arguments applied after version and schema checks. + Excludes manual arguments. +- `core_beta_opted_in`: resolved beta setting at launch; false if reading the setting failed. +- `core_version`: recorded Core release label from `coreSemver(inst)`, or null if unavailable. + Not proof of the live checkout's version. +- `app_version`: Desktop version, attached centrally by `src/main/lib/telemetry.ts`. +- `boot_id`: per-launch join key shared by the lifecycle events. Retries reuse the same key. + +`assets_enabled` describes an explicit launch argument, not service health. It does not detect a +future Core default that enables Assets without that argument, or prove that Assets initialized. +On schema-discovery failure, preserved manual arguments still count; arguments removed by a +successful schema check do not. Opting out suppresses grants, not manually supplied flags. + +The false cohort mixes opted-out, ungranted, version-ineligible and schema-ineligible launches. +Segment by Core version and opt-in before comparing. Recorded release labels can lag a modified +checkout or describe a base tag rather than an exact release, so use known pinned releases when +interpreting a version comparison. These fields do not identify every reason a grant was withheld. + +The normal telemetry consent gate still applies. No paths, filenames, asset names, prompts, model +metadata, or other user content are added. This follows the telemetry privacy rules documented in +[`src/main/lib/telemetry.ts`](../src/main/lib/telemetry.ts). + +## PostHog queries + +Boot duration by Desktop version, recorded Core version, beta opt-in and Assets argument: + +```sql +SELECT + properties.app_version AS desktop_version, + properties.core_version AS core_version, + properties.core_beta_opted_in AS beta_opted_in, + properties.assets_enabled AS assets_enabled, + count() AS completed_boots, + round(avg(toFloat(properties.boot_time_ms)), 0) AS mean_boot_ms, + round(quantile(0.5)(toFloat(properties.boot_time_ms)), 0) AS p50_boot_ms, + round(quantile(0.95)(toFloat(properties.boot_time_ms)), 0) AS p95_boot_ms +FROM events +WHERE event = 'comfy.desktop.comfyui.boot_completed' + AND timestamp >= now() - INTERVAL 14 DAY + AND properties.assets_enabled IS NOT NULL +GROUP BY desktop_version, core_version, beta_opted_in, assets_enabled +ORDER BY desktop_version DESC, core_version DESC, beta_opted_in DESC, assets_enabled DESC +``` + +Boot outcome with the same segmentation. Distinct boot IDs prevent retries from inflating counts: + +```sql +SELECT + properties.app_version AS desktop_version, + properties.core_version AS core_version, + properties.core_beta_opted_in AS beta_opted_in, + properties.assets_enabled AS assets_enabled, + uniqIf(properties.boot_id, event = 'comfy.desktop.comfyui.boot_started') AS boots_started, + uniqIf(properties.boot_id, event = 'comfy.desktop.comfyui.boot_completed') AS boots_completed, + uniqIf(properties.boot_id, event = 'comfy.desktop.comfyui.boot_failed') AS boots_failed, + round(100 * boots_completed / nullIf(boots_started, 0), 2) AS success_rate_pct +FROM events +WHERE event IN ( + 'comfy.desktop.comfyui.boot_started', + 'comfy.desktop.comfyui.boot_completed', + 'comfy.desktop.comfyui.boot_failed' +) + AND timestamp >= now() - INTERVAL 14 DAY + AND properties.assets_enabled IS NOT NULL +GROUP BY desktop_version, core_version, beta_opted_in, assets_enabled +ORDER BY desktop_version DESC, core_version DESC, beta_opted_in DESC, assets_enabled DESC +``` + +Use the first query as a duration trend and the second as an outcome table. Compare Assets cohorts +within the same Desktop version, Core version and opt-in segment. Keep unknown Core versions +separate. Rows predating these properties are excluded rather than counted as Assets off. + +Report cohort counts alongside durations and rates. Hardware, installation history, manual flags +and eligibility can still differ within a segment; do not call a difference an Assets-caused +regression. Recent launches may lack a terminal event, and cancellations are not boot failures. +The rolling window can also split a launch's start and completion across its boundary. + +## Glossary + +- **Cohort:** launches grouped by the recorded properties above. +- **Grant:** a managed beta argument authorized by the remote flag and local launch checks. +- **Core:** the ComfyUI Python process launched by Desktop. +- **Schema:** the supported command-line arguments discovered from Core. +- **PostHog / HogQL:** the event analytics service and its SQL query language. +- **p50 / p95:** the 50th and 95th percentiles of completed-boot duration. diff --git a/src/main/lib/ipc/sessionActions/launch.test.ts b/src/main/lib/ipc/sessionActions/launch.test.ts index a1b20285c..89f9586bb 100644 --- a/src/main/lib/ipc/sessionActions/launch.test.ts +++ b/src/main/lib/ipc/sessionActions/launch.test.ts @@ -912,6 +912,7 @@ describe('core beta report placement', () => { spawnArgs = [] launchHarness.grants = [HARNESS_GRANT] launchHarness.duringResourceAcquire = null + launchHarness.waitForPort = null launchHarness.spawn = (_cmd: unknown, args: unknown) => { spawnArgs = args as string[] return fakeChild() @@ -930,6 +931,12 @@ describe('core beta report placement', () => { ) => { events.push({ event, properties }) }) as unknown as typeof telemetry.emit) + vi.spyOn(telemetry, 'capture').mockImplementation((( + event: string, + properties?: Record + ) => { + events.push({ event, properties }) + }) as unknown as typeof telemetry.capture) }) afterEach(() => { @@ -1346,6 +1353,102 @@ describe('core beta report placement', () => { expect( events.filter((e) => e.event === 'comfy.desktop.comfyui.assets.seeder.scan_started') ).toHaveLength(60) + const bootEvents = events.filter((e) => e.event.startsWith('comfy.desktop.comfyui.boot_')) + expect(bootEvents.map((e) => e.event)).toEqual([ + 'comfy.desktop.comfyui.boot_started', + 'comfy.desktop.comfyui.boot_started', + 'comfy.desktop.comfyui.boot_completed' + ]) + expect(bootEvents.every((e) => e.properties?.assets_enabled === true)).toBe(true) + expect(bootEvents.map((e) => e.properties?.core_beta_flags)).toEqual([ + ['--enable-assets'], + ['--enable-assets'], + ['--enable-assets'] + ]) + for (const { properties } of bootEvents) { + expect(properties).toMatchObject({ core_beta_opted_in: true, core_version: '0.3.81' }) + } + }) + + it.each([ + // description, opted in, manual flag, discovery fails, flag supported, expected cohort + ['opted out without a flag', false, false, false, true, false], + ['opted out with a manual flag', false, true, false, true, true], + ['discovery fails with a manual flag', true, true, true, true, true], + ['discovery fails without a flag', true, false, true, true, false], + ['schema removes an unsupported manual flag', false, true, false, false, false], + ['opted in without a grant', true, false, false, true, false] + ] as const)( + 'tags boot arguments independently of grants: %s', + async (_description, optedIn, manualFlag, discoveryFails, supported, expected) => { + launchHarness.betaEnabled = optedIn + launchHarness.grants = [] + launchHarness.schemaThrows = discoveryFails + launchHarness.schemaNames = supported ? ['enable-assets', 'listen'] : ['listen'] + launchHarness.launchCommand = { + cmd: process.execPath, + args: [ + '-s', + path.join(installDir, 'ComfyUI', 'main.py'), + '--listen', + ...(manualFlag ? ['--enable-assets'] : []) + ], + cwd: installDir, + skipPortWait: false, + port: 48233 + } + launchHarness.waitForPort = async () => {} + + const res = await handleLaunch(ctxFor(`harness-assets-cohort-${_description}`)) + + expect(res.ok).toBe(true) + expect(spawnArgs.includes('--enable-assets')).toBe(expected) + const bootEvents = events.filter((e) => e.event.startsWith('comfy.desktop.comfyui.boot_')) + expect(bootEvents.map((e) => e.event)).toEqual([ + 'comfy.desktop.comfyui.boot_started', + 'comfy.desktop.comfyui.boot_completed' + ]) + for (const { properties } of bootEvents) { + expect(properties).toMatchObject({ + assets_enabled: expected, + core_beta_flags: [], + core_beta_opted_in: optedIn, + core_version: '0.3.81' + }) + } + } + ) + + it('keeps the applied Assets cohort on terminal boot failure', async () => { + launchHarness.launchCommand = { + cmd: process.execPath, + args: ['-s', path.join(installDir, 'ComfyUI', 'main.py'), '--listen'], + cwd: installDir, + skipPortWait: false, + port: 48234 + } + launchHarness.waitForPort = async () => { + throw new Error('boot timed out') + } + + const res = await handleLaunch(ctxFor('harness-assets-failed')) + + expect(res.ok).toBe(false) + const bootEvents = events.filter((e) => + ['comfy.desktop.comfyui.boot_started', 'comfy.desktop.comfyui.boot_failed'].includes(e.event) + ) + expect(bootEvents.map((e) => e.event)).toEqual([ + 'comfy.desktop.comfyui.boot_started', + 'comfy.desktop.comfyui.boot_failed' + ]) + expect(bootEvents.every((e) => e.properties?.assets_enabled === true)).toBe(true) + expect(bootEvents.map((e) => e.properties?.core_beta_flags)).toEqual([ + ['--enable-assets'], + ['--enable-assets'] + ]) + for (const { properties } of bootEvents) { + expect(properties).toMatchObject({ core_beta_opted_in: true, core_version: '0.3.81' }) + } }) it('still filters user args, injecting nothing, when the beta setting cannot be resolved', async () => { diff --git a/src/main/lib/ipc/sessionActions/launch.ts b/src/main/lib/ipc/sessionActions/launch.ts index 164dda849..f85c91598 100644 --- a/src/main/lib/ipc/sessionActions/launch.ts +++ b/src/main/lib/ipc/sessionActions/launch.ts @@ -790,6 +790,24 @@ async function runLaunch( } } + /** Launch-argument cohort, not confirmation that Core's Assets service initialized. + * Manual/source arguments count even when opted out or schema discovery fails; + * managed grants remain separate attribution. Called only after launchCmd exists. + * `app_version` is added centrally by telemetry.ts. */ + function bootCohort(): { + core_beta_flags: string[] + assets_enabled: boolean + core_beta_opted_in: boolean + core_version: string | null + } { + return { + core_beta_flags: coreBeta.applied.map((grant) => grant.arg), + assets_enabled: launchCmd.args?.includes('--enable-assets') === true, + core_beta_opted_in: coreBeta.optedIn, + core_version: coreSemver(inst) + } + } + // Migrate legacy envs/default/ → ComfyUI/.venv/ for standalone installs. if (inst.sourceId === 'standalone') { // Recover from an update/restore interrupted by a hard process kill (power @@ -1512,6 +1530,7 @@ async function runLaunch( installation_id: installationId, boot_id: bootId, variant: (inst.variant as string | undefined) ?? null, + ...bootCohort(), port_retry_count: portRetries, reboot_retry_count: rebootRetries }) @@ -1693,6 +1712,7 @@ async function runLaunch( installation_id: installationId, boot_id: bootId, variant: (inst.variant as string | undefined) ?? null, + ...bootCohort(), failed_phase: failedPhase, ...buildErrorFields(errorSource), error_tail: tail, @@ -1736,6 +1756,7 @@ async function runLaunch( installation_id: installationId, boot_id: bootId, variant: (inst.variant as string | undefined) ?? null, + ...bootCohort(), boot_time_ms: bootTimeMs, port_retry_count: portRetries, reboot_retry_count: rebootRetries