Skip to content

Conversation

@christian-byrne
Copy link
Contributor

@christian-byrne christian-byrne commented Nov 3, 2025

Summary

Manual backport of #6518 to the rh-test branch.

Deduplicates workflow run telemetry and keeps a single source of truth for execution while retaining click analytics and attributing initiator source.

  • Keep execution tracking in one place via trackWorkflowExecution()
  • Keep click analytics via trackRunButton(...)
  • Attribute initiator with trigger_source = 'button' | 'keybinding' | 'legacy_ui'
  • Remove pre-tracking from keybindings to avoid double/triple counting
  • Update legacy UI buttons to emit both click + execution events

Backport Notes

This backport required manual conflict resolution in:

  • src/components/actionbar/ComfyRunButton/ComfyQueueButton.vue - Added batchCount tracking and trigger_source metadata
  • src/composables/useCoreCommands.ts - Added error handling and execution tracking
  • src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts - Updated trackRunButton signature with trigger_source support

Additionally added:

  • trackUiButtonClicked method to TelemetryProvider interface
  • UiButtonClickMetadata type definition
  • UI_BUTTON_CLICKED event constant

All conflicts resolved intelligently to maintain the intent of the original PR while adapting to the rh-test branch codebase.

Original PR

Testing

  • ✅ Typecheck passed
  • ✅ Pre-commit hooks passed (lint, format)
  • ✅ All conflicts resolved

┆Issue is synchronized with this Notion page by Unito

christian-byrne and others added 2 commits November 2, 2025 20:08
… single execution event (#6518)

Deduplicates workflow run telemetry and keeps a single source of truth
for execution while retaining click analytics and attributing initiator
source.

- Keep execution tracking in one place via `trackWorkflowExecution()`
- Keep click analytics via `trackRunButton(...)`
- Attribute initiator with `trigger_source` = 'button' | 'keybinding' |
'legacy_ui'
- Remove pre‑tracking from keybindings to avoid double/triple counting
- Update legacy UI buttons to emit both click + execution events (they
bypass commands)

PR #6499 added tracking at multiple layers:
1) Keybindings tracked via a dedicated method and then executed a
command
2) Menu items tracked via a dedicated method and then executed a command
3) Commands also tracked execution

Because these ultimately trigger the same command path, this produced
duplicate (sometimes triplicate) events per user action and made it hard
to attribute initiator precisely.

- Remove redundant tracking from keybindings (and previous legacy menu
handler)
- Commands now emit both:
- `trackRunButton(...)` (click analytics, includes `trigger_source` when
provided)
- `trackWorkflowExecution()` (single execution start; includes the last
`trigger_source`)
- Legacy UI buttons (which call `app.queuePrompt(...)` directly) now
also emit both events with `trigger_source = 'legacy_ui'`
- Add `ExecutionTriggerSource` type and wire `trigger_source` through
provider so `EXECUTION_START` matches the most recent click intent

- `RUN_BUTTON_CLICKED` (click analytics)
  - Emitted when a run is initiated via:
    - Button: `trigger_source = 'button'`
    - Keybinding: `trigger_source = 'keybinding'`
    - Legacy UI: `trigger_source = 'legacy_ui'`
- `EXECUTION_START` (execution lifecycle)
- Emitted once per run at start; includes `trigger_source` matched to
the click intent above
- Paired with `EXECUTION_SUCCESS` / `EXECUTION_ERROR` from execution
handlers

- ✅ Accurate counts by removing duplicated run events
- ✅ Clear initiator attribution (button vs keybinding vs legacy UI)
- ✅ Separation of “intent” (click) vs. “lifecycle” (execution)
- ✅ Simpler implementation and maintenance

- `src/services/keybindingService.ts`: Route run commands with
`trigger_source = 'keybinding'`
- `src/components/actionbar/ComfyRunButton/ComfyQueueButton.vue`: Send
`trigger_source = 'button'` to commands
- `src/scripts/ui.ts`: Legacy queue buttons emit `trackRunButton({
trigger_source: 'legacy_ui' })` and `trackWorkflowExecution()`
- `src/composables/useCoreCommands.ts`: Commands emit
`trackRunButton(...)` + `trackWorkflowExecution()`; accept telemetry
metadata
- `src/platform/telemetry/types.ts`: Add `ExecutionTriggerSource` and
optional `trigger_source` in click + execution payloads
- `src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts`:
Carry `trigger_source` from click → execution and reset after use
- `src/stores/commandStore.ts`: Allow commands to receive args (for
telemetry metadata)
- `src/extensions/core/groupNode.ts`: Adjust command function signatures
to new execute signature

- Reverts the multi‑event approach from #6499
- Keeps `trackWorkflowExecution()` as the canonical execution event
while preserving click analytics and initiator attribution with
`trackRunButton(...)`

┆Issue is synchronized with this Notion page by Unito

---------

Co-authored-by: Christian Byrne <c.byrne@comfy.org>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
Co-authored-by: Benjamin Lu <benjaminlu1107@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Nov 3, 2025
@github-actions
Copy link

github-actions bot commented Nov 3, 2025

🎭 Playwright Test Results

Some tests failed

⏰ Completed at: 11/03/2025, 04:58:25 AM UTC

📈 Summary

  • Total Tests: 211
  • Passed: -108 ✅
  • Failed: 14 ❌
  • Flaky: 1 ⚠️
  • Skipped: 304 ⏭️

📊 Test Reports by Browser


🎉 Click on the links above to view detailed test results for each browser configuration.

@christian-byrne christian-byrne added the backport Backporting a PR onto a release candidate label Nov 3, 2025
@christian-byrne christian-byrne merged commit 56412a4 into rh-test Nov 3, 2025
18 of 24 checks passed
@christian-byrne christian-byrne deleted the backport-6518-to-rh-test branch November 3, 2025 04:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Backporting a PR onto a release candidate size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants