-
Notifications
You must be signed in to change notification settings - Fork 462
feat: New Template Library #7062
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
95d7207
01047c7
80139f3
885eeb4
77afd34
cf83d29
e7f85d8
4f23735
40bf249
65bcde7
9c1fe8c
476de50
74eb2d2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| # Template Ranking System | ||
|
|
||
| Usage-based ordering for workflow templates with position bias normalization. | ||
|
|
||
| Scores are pre-computed and normalized offline and shipped as static JSON (mirrors `sorted-custom-node-map.json` pattern for node search). | ||
|
|
||
| ## Sort Modes | ||
|
|
||
| | Mode | Formula | Description | | ||
| | -------------- | ------------------------------------------------ | ---------------------- | | ||
| | `recommended` | `usage × 0.5 + internal × 0.3 + freshness × 0.2` | Curated recommendation | | ||
| | `popular` | `usage × 0.9 + freshness × 0.1` | Pure user-driven | | ||
| | `newest` | Date sort | Existing | | ||
| | `alphabetical` | Name sort | Existing | | ||
|
|
||
| Freshness computed at runtime from `template.date`: `1.0 / (1 + daysSinceAdded / 90)`, min 0.1. | ||
|
|
||
| ## Data Files | ||
|
|
||
| **Usage scores** (generated from Mixpanel): | ||
|
|
||
| ```json | ||
| // In templates/index.json, add to any template: | ||
| { | ||
| "name": "some_template", | ||
| "usage": 1000, | ||
| ... | ||
| } | ||
| ``` | ||
|
|
||
| **Search rank** (set per-template in workflow_templates repo): | ||
|
|
||
| ```json | ||
| // In templates/index.json, add to any template: | ||
| { | ||
| "name": "some_template", | ||
| "searchRank": 8, // Scale 1-10, default 5 | ||
| ... | ||
| } | ||
| ``` | ||
|
|
||
| | searchRank | Effect | | ||
| | ---------- | ---------------------------- | | ||
| | 1-4 | Demote (bury in results) | | ||
| | 5 | Neutral (default if not set) | | ||
| | 6-10 | Promote (boost in results) | | ||
|
|
||
| ## Position Bias Correction | ||
|
|
||
| Raw usage reflects true preference AND UI position bias. We use linear interpolation: | ||
|
|
||
| ``` | ||
| correction = 1 + (position - 1) / (maxPosition - 1) | ||
| normalizedUsage = rawUsage × correction | ||
| ``` | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| | Position | Boost | | ||
| | -------- | ----- | | ||
| | 1 | 1.0× | | ||
| | 50 | 1.28× | | ||
| | 100 | 1.57× | | ||
| | 175 | 2.0× | | ||
|
|
||
| Templates buried at the bottom get up to 2× boost to compensate for reduced visibility. | ||
|
|
||
| --- | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -175,6 +175,7 @@ | |
| <!-- Actual Template Cards --> | ||
| <CardContainer | ||
| v-for="template in isLoading ? [] : displayTemplates" | ||
| v-show="isTemplateVisibleOnDistribution(template)" | ||
| :key="template.name" | ||
| ref="cardRefs" | ||
| size="compact" | ||
|
|
@@ -405,6 +406,8 @@ import { useTelemetry } from '@/platform/telemetry' | |
| import { useTemplateWorkflows } from '@/platform/workflow/templates/composables/useTemplateWorkflows' | ||
| import { useWorkflowTemplatesStore } from '@/platform/workflow/templates/repositories/workflowTemplatesStore' | ||
| import type { TemplateInfo } from '@/platform/workflow/templates/types/template' | ||
| import { TemplateIncludeOnDistributionEnum } from '@/platform/workflow/templates/types/template' | ||
| import { useSystemStatsStore } from '@/stores/systemStatsStore' | ||
| import type { NavGroupData, NavItemData } from '@/types/navTypes' | ||
| import { OnCloseKey } from '@/types/widgetTypes' | ||
| import { createGridStyle } from '@/utils/gridUtil' | ||
|
|
@@ -423,6 +426,30 @@ onMounted(() => { | |
| sessionStartTime.value = Date.now() | ||
| }) | ||
|
|
||
| const systemStatsStore = useSystemStatsStore() | ||
|
|
||
| const distributions = computed(() => { | ||
| // eslint-disable-next-line no-undef | ||
| switch (__DISTRIBUTION__) { | ||
| case 'cloud': | ||
| return [TemplateIncludeOnDistributionEnum.Cloud] | ||
| case 'localhost': | ||
| return [TemplateIncludeOnDistributionEnum.Local] | ||
| case 'desktop': | ||
| default: | ||
| if (systemStatsStore.systemStats?.system.os === 'darwin') { | ||
| return [ | ||
| TemplateIncludeOnDistributionEnum.Desktop, | ||
| TemplateIncludeOnDistributionEnum.Mac | ||
| ] | ||
| } | ||
| return [ | ||
| TemplateIncludeOnDistributionEnum.Desktop, | ||
| TemplateIncludeOnDistributionEnum.Windows | ||
| ] | ||
| } | ||
| }) | ||
|
Comment on lines
+429
to
+451
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, find the useSystemStatsStore implementation
find src -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs rg -l "useSystemStatsStore|export.*systemStatsStore" | head -10Repository: Comfy-Org/ComfyUI_frontend Length of output: 438 🏁 Script executed: # Check the component file around the lines in question
cat -n src/components/custom/widget/WorkflowTemplateSelectorDialog.vue | sed -n '420,460p'Repository: Comfy-Org/ComfyUI_frontend Length of output: 1432 🏁 Script executed: # Search for where systemStats is loaded/initialized in the store
rg -nP "systemStats\s*=|systemStats\s*\:" src --type=ts --type=tsx -A 3 | head -80Repository: Comfy-Org/ComfyUI_frontend Length of output: 97 🏁 Script executed: # Search for initialization of systemStats in stores
fd -e ts -e tsx -e vue src | xargs rg -l "systemStats" | grep -E "store|composable"Repository: Comfy-Org/ComfyUI_frontend Length of output: 608 🏁 Script executed: # Examine the systemStatsStore implementation
cat src/stores/systemStatsStore.tsRepository: Comfy-Org/ComfyUI_frontend Length of output: 1736 🏁 Script executed: # Check the test file for usage patterns and expected behavior
cat tests-ui/tests/store/systemStatsStore.test.tsRepository: Comfy-Org/ComfyUI_frontend Length of output: 10381 🏁 Script executed: # Search for where systemStatsStore is initialized at app startup
rg -nP "useSystemStatsStore|systemStatsStore" src/main.ts src/App.vue src/stores --type=ts -B 2 -A 5Repository: Comfy-Org/ComfyUI_frontend Length of output: 1823 Handle The Add a guard using the store's const distributions = computed(() => {
switch (__DISTRIBUTION__) {
case 'cloud':
return [TemplateIncludeOnDistributionEnum.Cloud]
case 'localhost':
return [TemplateIncludeOnDistributionEnum.Local]
case 'desktop':
default:
if (!systemStatsStore.isInitialized) {
return [TemplateIncludeOnDistributionEnum.Desktop]
}
if (systemStatsStore.systemStats?.system.os === 'darwin') {
return [
TemplateIncludeOnDistributionEnum.Desktop,
TemplateIncludeOnDistributionEnum.Mac
]
}
return [
TemplateIncludeOnDistributionEnum.Desktop,
TemplateIncludeOnDistributionEnum.Windows
]
}
})🤖 Prompt for AI Agents |
||
|
|
||
| // Wrap onClose to track session end | ||
| const onClose = () => { | ||
| if (isCloud) { | ||
|
|
@@ -511,6 +538,9 @@ const allTemplates = computed(() => { | |
| return workflowTemplatesStore.enhancedTemplates | ||
| }) | ||
|
|
||
| // Navigation | ||
| const selectedNavItem = ref<string | null>('all') | ||
|
|
||
| // Filter templates based on selected navigation item | ||
| const navigationFilteredTemplates = computed(() => { | ||
| if (!selectedNavItem.value) { | ||
|
|
@@ -536,6 +566,36 @@ const { | |
| resetFilters | ||
| } = useTemplateFiltering(navigationFilteredTemplates) | ||
|
|
||
| /** | ||
| * Coordinates state between the selected navigation item and the sort order to | ||
| * create deterministic, predictable behavior. | ||
| * @param source The origin of the change ('nav' or 'sort'). | ||
| */ | ||
| const coordinateNavAndSort = (source: 'nav' | 'sort') => { | ||
| const isPopularNav = selectedNavItem.value === 'popular' | ||
| const isPopularSort = sortBy.value === 'popular' | ||
|
|
||
| if (source === 'nav') { | ||
| if (isPopularNav && !isPopularSort) { | ||
| // When navigating to 'Popular' category, automatically set sort to 'Popular'. | ||
| sortBy.value = 'popular' | ||
| } else if (!isPopularNav && isPopularSort) { | ||
| // When navigating away from 'Popular' category while sort is 'Popular', reset sort to default. | ||
| sortBy.value = 'default' | ||
| } | ||
| } else if (source === 'sort') { | ||
| // When sort is changed away from 'Popular' while in the 'Popular' category, | ||
| // reset the category to 'All Templates' to avoid a confusing state. | ||
| if (isPopularNav && !isPopularSort) { | ||
| selectedNavItem.value = 'all' | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Watch for changes from the two sources ('nav' and 'sort') and trigger the coordinator. | ||
| watch(selectedNavItem, () => coordinateNavAndSort('nav')) | ||
| watch(sortBy, () => coordinateNavAndSort('sort')) | ||
|
|
||
| // Convert between string array and object array for MultiSelect component | ||
| const selectedModelObjects = computed({ | ||
| get() { | ||
|
|
@@ -578,9 +638,6 @@ const cardRefs = ref<HTMLElement[]>([]) | |
| // Force re-render key for templates when sorting changes | ||
| const templateListKey = ref(0) | ||
|
|
||
| // Navigation | ||
| const selectedNavItem = ref<string | null>('all') | ||
|
|
||
| // Search text for model filter | ||
| const modelSearchText = ref<string>('') | ||
|
|
||
|
|
@@ -645,11 +702,19 @@ const runsOnFilterLabel = computed(() => { | |
|
|
||
| // Sort options | ||
| const sortOptions = computed(() => [ | ||
| { name: t('templateWorkflows.sort.newest', 'Newest'), value: 'newest' }, | ||
| { | ||
| name: t('templateWorkflows.sort.default', 'Default'), | ||
| value: 'default' | ||
| }, | ||
| { | ||
| name: t('templateWorkflows.sort.recommended', 'Recommended'), | ||
| value: 'recommended' | ||
| }, | ||
| { | ||
| name: t('templateWorkflows.sort.popular', 'Popular'), | ||
| value: 'popular' | ||
| }, | ||
| { name: t('templateWorkflows.sort.newest', 'Newest'), value: 'newest' }, | ||
| { | ||
| name: t('templateWorkflows.sort.vramLowToHigh', 'VRAM Usage (Low to High)'), | ||
| value: 'vram-low-to-high' | ||
|
|
@@ -750,7 +815,7 @@ const pageTitle = computed(() => { | |
| // Initialize templates loading with useAsyncState | ||
| const { isLoading } = useAsyncState( | ||
| async () => { | ||
| // Run both operations in parallel for better performance | ||
| // Run all operations in parallel for better performance | ||
| await Promise.all([ | ||
| loadTemplates(), | ||
| workflowTemplatesStore.loadWorkflowTemplates() | ||
|
|
@@ -763,6 +828,14 @@ const { isLoading } = useAsyncState( | |
| } | ||
| ) | ||
|
|
||
| const isTemplateVisibleOnDistribution = (template: TemplateInfo) => { | ||
| return (template.includeOnDistributions?.length ?? 0) > 0 | ||
| ? distributions.value.some((d) => | ||
| template.includeOnDistributions?.includes(d) | ||
| ) | ||
| : true | ||
| } | ||
|
|
||
| onBeforeUnmount(() => { | ||
| cardRefs.value = [] // Release DOM refs | ||
| }) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import { createPinia, setActivePinia } from 'pinia' | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' | ||
| import { nextTick, ref } from 'vue' | ||
|
|
||
|
|
@@ -19,10 +20,22 @@ const defaultSettingStore = { | |
| set: vi.fn().mockResolvedValue(undefined) | ||
| } | ||
|
|
||
| const defaultRankingStore = { | ||
| computeDefaultScore: vi.fn(() => 0), | ||
| computePopularScore: vi.fn(() => 0), | ||
| getUsageScore: vi.fn(() => 0), | ||
| computeFreshness: vi.fn(() => 0.5), | ||
| isLoaded: { value: false } | ||
| } | ||
|
Comment on lines
+23
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider adding test coverage for 'recommended' and 'popular' sort modes. The mock always returns constant values (0 for scores, 0.5 for freshness), which means templates will have identical scores. While this works for basic testing, there's no dedicated test coverage verifying the 'recommended' and 'popular' sorting behavior actually uses the ranking store correctly. Consider adding tests that configure the mock to return different scores for different templates to verify sorting order. 🤖 Prompt for AI Agents |
||
|
|
||
| vi.mock('@/platform/settings/settingStore', () => ({ | ||
| useSettingStore: vi.fn(() => defaultSettingStore) | ||
| })) | ||
|
|
||
| vi.mock('@/stores/templateRankingStore', () => ({ | ||
| useTemplateRankingStore: vi.fn(() => defaultRankingStore) | ||
| })) | ||
|
|
||
| vi.mock('@/platform/telemetry', () => ({ | ||
| useTelemetry: vi.fn(() => ({ | ||
| trackTemplateFilterChanged: vi.fn() | ||
|
|
@@ -34,6 +47,7 @@ const { useTemplateFiltering } = | |
|
|
||
| describe('useTemplateFiltering', () => { | ||
| beforeEach(() => { | ||
| setActivePinia(createPinia()) | ||
| vi.clearAllMocks() | ||
| }) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
The fragile selector remains unresolved.
Changing
nth-child(2)tonth-child(3)fixes the immediate breakage but perpetuates the same maintenance issue. When navigation structure changes again, this test will break again. The previous review already suggested robust alternatives (data-testid, role-based selectors, or scoped getByText), which would prevent this class of breakage.Based on learnings, prefer specific selectors in browser tests and favor accessible properties over structural selectors like nth-child.