feat(telemetry): help center and workflow creation#6505
feat(telemetry): help center and workflow creation#6505christian-byrne merged 4 commits intomainfrom
Conversation
dosubot
bot
added
the
size:L
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 11/01/2025, 06:48:40 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 11/01/2025, 07:03:16 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.27 MB (baseline 3.27 MB) • ⚪ 0 BMain entry bundles and manifests
Status: 2 added / 2 removed Graph Workspace — 724 kB (baseline 723 kB) • 🔴 +265 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 8.18 kB (baseline 8.18 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 295 kB (baseline 295 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 12.3 kB (baseline 12.3 kB) • ⚪ 0 BReusable component library chunks
Status: 1 added / 1 removed Data & Services — 11.4 kB (baseline 11.4 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 1 added / 1 removed Utilities & Hooks — 1.07 kB (baseline 1.07 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 5.32 MB (baseline 5.32 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 2.55 MB (baseline 2.55 MB) • ⚪ 0 BBundles that do not match a named category
|
christian-byrne
added
the
claude-review
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| ) | ||
|
|
||
| const showContactSupport = async () => { | ||
| if (isCloud) { |
There was a problem hiding this comment.
[quality] medium Priority
Issue: Inconsistent conditional telemetry tracking pattern
Context: Some functions check isCloud before calling telemetry, others don't, creating inconsistent behavior and potential errors in non-cloud environments
Suggestion: Use consistent pattern - either centralize the cloud check in the telemetry provider itself, or ensure all telemetry calls are consistently wrapped with isCloud checks
| category: 'essentials' as const, | ||
| function: () => workflowService.loadBlankWorkflow() | ||
| function: async () => { | ||
| const previousWorkflowHadNodes = app.graph._nodes.length > 0 |
There was a problem hiding this comment.
[quality] medium Priority
Issue: Workflow creation tracking logic may not be accurate in all scenarios
Context: The check for previousWorkflowHadNodes might miss edge cases like workflows with only IO nodes or subgraphs, potentially providing misleading analytics data
Suggestion: Consider more robust workflow state detection, possibly checking for meaningful workflow content beyond just node count
There was a problem hiding this comment.
we can improve this if we see these problems in the data.
|
|
||
| trackWorkflowCreated(metadata: WorkflowCreatedMetadata): void { | ||
| this.trackEvent(TelemetryEvents.WORKFLOW_CREATED, metadata) | ||
| } |
There was a problem hiding this comment.
[performance] medium Priority
Issue: Complex execution context calculation runs on every telemetry call
Context: The getExecutionContext method performs heavy graph traversal and multiple store lookups on each call, which could impact performance during frequent tracking events
Suggestion: Consider caching execution context or calculating it only when the graph changes, rather than on every telemetry event
There was a problem hiding this comment.
Comprehensive PR Review
This review is generated by Claude. It may not always be accurate, as with human reviewers. If you believe that any of the comments are invalid or incorrect, please state why for each. For others, please implement the changes in one way or another.
Review Summary
PR: feat(telemetry): help center and workflow creation (#6505)
Impact: 202 additions, 3 deletions across 7 files
Issue Distribution
- Critical: 0
- High: 0
- Medium: 3
- Low: 3
Category Breakdown
- Architecture: 0 issues
- Security: 0 issues
- Performance: 2 issues
- Code Quality: 4 issues
Key Findings
Architecture & Design
The PR follows established telemetry patterns and integrates well with the existing infrastructure. The conditional cloud-only tracking is appropriate for the distribution model. The separation of concerns between tracking methods and metadata types is well-structured.
Security Considerations
No security issues identified. The telemetry system properly handles the cloud/OSS distribution boundaries and doesn't expose sensitive data. The conditional compilation approach ensures OSS builds remain clean.
Performance Impact
Two moderate performance concerns identified:
- Complex execution context calculation - The getExecutionContext method performs heavy graph traversal on every telemetry call, which could impact performance during frequent events
- Time-based tracking overhead - Help center time tracking calculations on every unmount could create unnecessary computational overhead
Code Quality Issues
Several quality improvements identified:
- Inconsistent conditional patterns - Some telemetry calls check isCloud while others don't, creating potential inconsistency
- Missing error handling - Telemetry calls aren't wrapped in error handling, potentially causing UI disruption
- Workflow state detection - The logic for detecting previous workflow state could be more robust for edge cases
- Type completeness - Union types for tracking sources may not cover all future scenarios
Positive Observations
- Well-structured metadata types with clear documentation
- Appropriate use of existing patterns following established telemetry conventions
- Good separation of concerns between different tracking events
- Proper TypeScript typing throughout the implementation
- Clear event naming conventions following the app: prefix pattern
- Consistent parameter structure across similar tracking methods
References
Next Steps
- Address medium priority issues for consistency and robustness
- Consider performance optimization for execution context calculation
- Implement consistent error handling patterns for telemetry calls
- Document source tracking patterns to prevent future inconsistencies
This is a comprehensive automated review. For architectural decisions requiring human judgment, please request additional manual review.
github-actions
bot
removed
the
claude-review
christian-byrne
added
needs-backport
- Merge main into telemetry/help-center-and-workflow-creation - Resolved conflict in CreditsPanel.vue by keeping both telemetry tracking and subscription checking - Fixed vue-i18n mock in useSubscriptionActions.test.ts to include createI18n export using importOriginal pattern
|
Accidentally pushed commit from old GH account. Just fixed tests. |
|
I'm just making one change then merging. |
|
Go for it. |
Remove isCloud conditional checks around telemetry tracking calls since useTelemetry() already returns null in OSS builds. This simplifies the code while maintaining the same behavior through tree-shaking. Affected files: - HelpCenterMenuContent.vue (3 locations) - ErrorDialogContent.vue (1 location) - CreditsPanel.vue (1 location) - ComfyQueueButton.vue (1 location) - useCoreCommands.ts (6 locations) Also removed unused isCloud imports from files where it was only used for telemetry checks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
christian-byrne
deleted the
telemetry/help-center-and-workflow-creation
branch
|
@arjansingh Backport to Please manually cherry-pick commit Conflicting files
|
|
i'm gonna bring this in with the other cherry pick i am working on. |
For Cloud distribution: 1. Track help center usage 2. Track workflow creation ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6505-feat-telemetry-help-center-and-workflow-creation-29e6d73d36508185af8ccbf19d5af9e7) by [Unito](https://www.unito.io) --------- Co-authored-by: Christian Byrne <chrbyrne96@gmail.com> Co-authored-by: Christian Byrne <cbyrne@comfy.org> Co-authored-by: Claude <noreply@anthropic.com>
For Cloud distribution: 1. Track help center usage 2. Track workflow creation ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6505-feat-telemetry-help-center-and-workflow-creation-29e6d73d36508185af8ccbf19d5af9e7) by [Unito](https://www.unito.io) --------- Co-authored-by: Christian Byrne <chrbyrne96@gmail.com> Co-authored-by: Christian Byrne <cbyrne@comfy.org> Co-authored-by: Claude <noreply@anthropic.com>
For Cloud distribution: 1. Track help center usage 2. Track workflow creation ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6505-feat-telemetry-help-center-and-workflow-creation-29e6d73d36508185af8ccbf19d5af9e7) by [Unito](https://www.unito.io) --------- Co-authored-by: Christian Byrne <chrbyrne96@gmail.com> Co-authored-by: Christian Byrne <cbyrne@comfy.org> Co-authored-by: Claude <noreply@anthropic.com>
## Summary Resolves issues with #6503 ## Changes - Backport #6400 - Fix circular dependency issue - Backport #6505 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6522-rh-test-Telemetry-Backports-29e6d73d365081258d10c08299bde69b) by [Unito](https://www.unito.io) --------- Co-authored-by: Christian Byrne <cbyrne@comfy.org> Co-authored-by: Christian Byrne <chrbyrne96@gmail.com> Co-authored-by: Claude <noreply@anthropic.com>
3 participants
Summary
For Cloud distribution:
┆Issue is synchronized with this Notion page by Unito