mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-06 05:30:08 +00:00
refactor: remove sampling logic for small nightly cohort
- Remove sampleRate config option, getUserSamplingId, isUserInSample - Simplifies eligibility to ~2k nightly users without random sampling - Addresses coderabbitai review: hash edge case bug, YAGNI for cohort size - Document featureId must remain static after initialization
This commit is contained in:
@@ -2,7 +2,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const SURVEY_STATE_KEY = 'Comfy.SurveyState'
|
||||
const FEATURE_USAGE_KEY = 'Comfy.FeatureUsage'
|
||||
const USER_SAMPLING_ID_KEY = 'Comfy.SurveyUserId'
|
||||
|
||||
const mockIsNightly = vi.hoisted(() => ({ value: true }))
|
||||
const mockIsCloud = vi.hoisted(() => ({ value: false }))
|
||||
@@ -263,62 +262,6 @@ describe('useSurveyEligibility', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('sampling', () => {
|
||||
it('creates stable user sampling ID', async () => {
|
||||
setFeatureUsage('test-feature', 5)
|
||||
|
||||
const { useSurveyEligibility } = await import('./useSurveyEligibility')
|
||||
const { isEligible } = useSurveyEligibility({
|
||||
...defaultConfig,
|
||||
sampleRate: 0.5
|
||||
})
|
||||
|
||||
// Access isEligible to trigger sampling check
|
||||
void isEligible.value
|
||||
|
||||
const userId = localStorage.getItem(USER_SAMPLING_ID_KEY)
|
||||
expect(userId).toBeTruthy()
|
||||
expect(userId).toMatch(
|
||||
/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/
|
||||
)
|
||||
})
|
||||
|
||||
it('reuses existing user sampling ID', async () => {
|
||||
const existingId = '12345678-1234-1234-1234-123456789012'
|
||||
localStorage.setItem(USER_SAMPLING_ID_KEY, existingId)
|
||||
setFeatureUsage('test-feature', 5)
|
||||
|
||||
const { useSurveyEligibility } = await import('./useSurveyEligibility')
|
||||
useSurveyEligibility({ ...defaultConfig, sampleRate: 0.5 })
|
||||
|
||||
expect(localStorage.getItem(USER_SAMPLING_ID_KEY)).toBe(existingId)
|
||||
})
|
||||
|
||||
it('sample rate of 0 excludes all users', async () => {
|
||||
setFeatureUsage('test-feature', 5)
|
||||
|
||||
const { useSurveyEligibility } = await import('./useSurveyEligibility')
|
||||
const { isEligible } = useSurveyEligibility({
|
||||
...defaultConfig,
|
||||
sampleRate: 0
|
||||
})
|
||||
|
||||
expect(isEligible.value).toBe(false)
|
||||
})
|
||||
|
||||
it('sample rate of 1 includes all users', async () => {
|
||||
setFeatureUsage('test-feature', 5)
|
||||
|
||||
const { useSurveyEligibility } = await import('./useSurveyEligibility')
|
||||
const { isEligible } = useSurveyEligibility({
|
||||
...defaultConfig,
|
||||
sampleRate: 1
|
||||
})
|
||||
|
||||
expect(isEligible.value).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('config values', () => {
|
||||
it('exposes delayMs from config', async () => {
|
||||
const { useSurveyEligibility } = await import('./useSurveyEligibility')
|
||||
|
||||
@@ -8,11 +8,11 @@ import { useFeatureUsageTracker } from './useFeatureUsageTracker'
|
||||
|
||||
/** @public */
|
||||
export interface FeatureSurveyConfig {
|
||||
/** Feature identifier. Must remain static after initialization. */
|
||||
featureId: string
|
||||
typeformId: string
|
||||
triggerThreshold?: number
|
||||
delayMs?: number
|
||||
sampleRate?: number
|
||||
enabled?: boolean
|
||||
}
|
||||
|
||||
@@ -26,7 +26,6 @@ const STORAGE_KEY = 'Comfy.SurveyState'
|
||||
const GLOBAL_COOLDOWN_MS = 14 * 24 * 60 * 60 * 1000 // 14 days
|
||||
const DEFAULT_THRESHOLD = 3
|
||||
const DEFAULT_DELAY_MS = 5000
|
||||
const DEFAULT_SAMPLE_RATE = 1
|
||||
|
||||
function getStorageState() {
|
||||
return useStorage<SurveyState>(STORAGE_KEY, {
|
||||
@@ -50,9 +49,6 @@ export function useSurveyEligibility(
|
||||
const delayMs = computed(
|
||||
() => resolvedConfig.value.delayMs ?? DEFAULT_DELAY_MS
|
||||
)
|
||||
const sampleRate = computed(
|
||||
() => resolvedConfig.value.sampleRate ?? DEFAULT_SAMPLE_RATE
|
||||
)
|
||||
const enabled = computed(() => resolvedConfig.value.enabled ?? true)
|
||||
|
||||
const isNightlyLocalhost = computed(() => isNightly && !isCloud && !isDesktop)
|
||||
@@ -78,11 +74,6 @@ export function useSurveyEligibility(
|
||||
if (isInGlobalCooldown.value) return false
|
||||
if (hasOptedOut.value) return false
|
||||
|
||||
if (sampleRate.value < 1) {
|
||||
const userId = getUserSamplingId()
|
||||
if (!isUserInSample(userId, sampleRate.value)) return false
|
||||
}
|
||||
|
||||
return true
|
||||
})
|
||||
|
||||
@@ -116,25 +107,3 @@ export function useSurveyEligibility(
|
||||
resetState
|
||||
}
|
||||
}
|
||||
|
||||
const USER_SAMPLING_ID_KEY = 'Comfy.SurveyUserId'
|
||||
|
||||
function getUserSamplingId(): string {
|
||||
let id = localStorage.getItem(USER_SAMPLING_ID_KEY)
|
||||
if (!id) {
|
||||
id = crypto.randomUUID()
|
||||
localStorage.setItem(USER_SAMPLING_ID_KEY, id)
|
||||
}
|
||||
return id
|
||||
}
|
||||
|
||||
function isUserInSample(userId: string, sampleRate: number): boolean {
|
||||
let hash = 0
|
||||
for (let i = 0; i < userId.length; i++) {
|
||||
const char = userId.charCodeAt(i)
|
||||
hash = (hash << 5) - hash + char
|
||||
hash = hash & hash
|
||||
}
|
||||
const normalized = Math.abs(hash) / 0x7fffffff
|
||||
return normalized < sampleRate
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user