From 6b9676bfb5aa13780167bfe9d81f37775a3e8b63 Mon Sep 17 00:00:00 2001 From: Richard Yu Date: Thu, 4 Dec 2025 16:48:52 -0800 Subject: [PATCH] chore: Remove migration plan doc --- docs/JOBS_API_MIGRATION_PLAN.md | 201 ------------------ src/composables/queue/useJobMenu.ts | 7 +- .../remote/comfyui/history/reconciliation.ts | 2 +- src/scripts/api.ts | 5 +- src/stores/queueStore.ts | 11 +- .../stores/queueStore.loadWorkflow.test.ts | 9 +- 6 files changed, 24 insertions(+), 211 deletions(-) delete mode 100644 docs/JOBS_API_MIGRATION_PLAN.md diff --git a/docs/JOBS_API_MIGRATION_PLAN.md b/docs/JOBS_API_MIGRATION_PLAN.md deleted file mode 100644 index 27801bd37..000000000 --- a/docs/JOBS_API_MIGRATION_PLAN.md +++ /dev/null @@ -1,201 +0,0 @@ -# Jobs API Migration Plan - -## Overview - -This document outlines the strategy for breaking up the Jobs API migration PR (#7125) into smaller, reviewable chunks. - -## Current State - -The PR migrates the frontend from legacy `/history` and `/queue` endpoints to the unified `/jobs` API. This involves: - -- **24 source files** changed -- **16 test files** changed -- Core changes to `TaskItemImpl` in `queueStore.ts` - -## Dependency Analysis - -``` -jobTypes.ts (types) - ↓ -fetchJobs.ts (fetchers) - ↓ -api.ts (API layer) - ↓ -queueStore.ts (TaskItemImpl rewrite) - ↓ -┌───────────────────────────────────────┐ -│ All consumers must update together: │ -│ - useJobList.ts │ -│ - useJobMenu.ts │ -│ - useResultGallery.ts │ -│ - useJobErrorReporting.ts │ -│ - JobGroupsList.vue │ -│ - assetsStore.ts │ -│ - reconciliation.ts │ -└───────────────────────────────────────┘ -``` - -## Proposed PR Split - -### PR 1: Jobs API Infrastructure (Foundation) -**Status**: Can merge independently -**Risk**: Low - purely additive - -Files: -``` -src/platform/remote/comfyui/jobs/types/jobTypes.ts (new) -src/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts (new) -src/platform/remote/comfyui/jobs/index.ts (new) -src/scripts/api.ts (add new methods) -``` - -Changes: -- Add Zod schemas for Jobs API response types -- Add `fetchQueue()` and `fetchHistory()` functions using `/jobs` endpoint -- Add `getQueue()` and `getHistory()` methods to ComfyApi class -- Export types from barrel file - -Tests: -- Unit tests for fetchers -- Integration tests for API methods - -**Why separate?** This is purely additive. The new code exists alongside the old code without breaking anything. Can be reviewed and merged first. - ---- - -### PR 2: Core Migration (TaskItemImpl + Consumers) -**Status**: Requires PR 1 -**Risk**: Medium - breaking changes to core data model - -Files: -``` -src/stores/queueStore.ts (rewrite TaskItemImpl) -src/schemas/apiSchema.ts (type updates) -src/composables/queue/useJobList.ts (use new TaskItemImpl) -src/composables/queue/useJobMenu.ts (use new TaskItemImpl) -src/composables/queue/useResultGallery.ts (use new TaskItemImpl) -src/components/queue/job/useJobErrorReporting.ts (use new TaskItemImpl) -src/components/queue/job/JobGroupsList.vue (fix workflowId access) -src/components/queue/QueueProgressOverlay.vue (if needed) -src/stores/assetsStore.ts (use JobListItem) -src/platform/remote/comfyui/history/reconciliation.ts (work with JobListItem) -src/platform/workflow/cloud/getWorkflowFromHistory.ts (use fetchJobDetail) -src/scripts/ui.ts (type fix) -``` - -Changes: -- Rewrite `TaskItemImpl` to wrap `JobListItem` instead of legacy tuple format -- Update all getters to derive from job properties -- Update all consumers to use new property names -- Update reconciliation to work with `JobListItem[]` - -Tests: -- All queue-related tests -- queueStore tests -- Integration tests - -**Why together?** These changes are tightly coupled. `TaskItemImpl` API changes break all consumers, so they must be updated atomically. - ---- - -### PR 3: Cleanup Legacy Code -**Status**: Requires PR 2 -**Risk**: Low - removing unused code - -Files to DELETE: -``` -src/platform/remote/comfyui/history/adapters/v2ToV1Adapter.ts -src/platform/remote/comfyui/history/fetchers/fetchHistoryV1.ts -src/platform/remote/comfyui/history/fetchers/fetchHistoryV2.ts -src/platform/remote/comfyui/history/types/historyV1Types.ts -src/platform/remote/comfyui/history/types/historyV2Types.ts -tests-ui/fixtures/historyFixtures.ts -tests-ui/fixtures/historySortingFixtures.ts -+ related test files -``` - -Files to MODIFY: -``` -src/platform/remote/comfyui/history/index.ts (remove old exports) -src/platform/remote/comfyui/history/types/index.ts (remove old exports) -``` - -**Why separate?** Deletion is low-risk but should be done after confirming the new code works in production. Allows rollback if issues are found. - ---- - -## Alternative: Feature Flag Approach - -If the above split is still too risky, consider: - -1. Add feature flag `useJobsApi` (default: false) -2. Keep both code paths in TaskItemImpl -3. Gradually roll out via feature flag -4. Remove old code path after validation - -This is more complex but allows incremental rollout. - ---- - -## Recommended Order - -1. **PR 1** → Merge first (no risk) -2. **PR 2** → Merge after PR 1 (main migration) -3. **PR 3** → Merge after validating PR 2 in production - -## Current PR Status - -The current PR (#7125) contains PR 1 + PR 2 combined. To split: - -1. Create new branch from main -2. Cherry-pick only the Jobs API infrastructure commits -3. Open PR 1 -4. Rebase current branch on PR 1 after merge -5. Current branch becomes PR 2 - ---- - -## Files by PR - -### PR 1 Files (8 files) -``` -src/platform/remote/comfyui/jobs/types/jobTypes.ts -src/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts -src/platform/remote/comfyui/jobs/index.ts -src/scripts/api.ts -+ 4 test files -``` - -### PR 2 Files (~28 files) -``` -src/stores/queueStore.ts -src/stores/assetsStore.ts -src/schemas/apiSchema.ts -src/scripts/ui.ts -src/composables/queue/useJobList.ts -src/composables/queue/useJobMenu.ts -src/composables/queue/useResultGallery.ts -src/components/queue/job/useJobErrorReporting.ts -src/components/queue/job/JobGroupsList.vue -src/components/queue/job/JobDetailsPopover.stories.ts -src/components/queue/QueueProgressOverlay.vue -src/platform/remote/comfyui/history/reconciliation.ts -src/platform/remote/comfyui/history/index.ts -src/platform/workflow/cloud/getWorkflowFromHistory.ts -src/platform/workflow/cloud/index.ts -browser_tests/fixtures/ComfyPage.ts -browser_tests/fixtures/utils/taskHistory.ts -+ ~12 test files -``` - -### PR 3 Files (~12 files to delete) -``` -DELETE: src/platform/remote/comfyui/history/adapters/* -DELETE: src/platform/remote/comfyui/history/fetchers/fetchHistoryV1.ts -DELETE: src/platform/remote/comfyui/history/fetchers/fetchHistoryV2.ts -DELETE: src/platform/remote/comfyui/history/types/historyV1Types.ts -DELETE: src/platform/remote/comfyui/history/types/historyV2Types.ts -DELETE: tests-ui/fixtures/history*.ts -DELETE: related test files -MODIFY: index.ts files to remove exports -``` diff --git a/src/composables/queue/useJobMenu.ts b/src/composables/queue/useJobMenu.ts index a88a9cebe..a668a5a97 100644 --- a/src/composables/queue/useJobMenu.ts +++ b/src/composables/queue/useJobMenu.ts @@ -6,7 +6,10 @@ import { useCopyToClipboard } from '@/composables/useCopyToClipboard' import { st, t } from '@/i18n' import { mapTaskOutputToAssetItem } from '@/platform/assets/composables/media/assetMappers' import { useMediaAssetActions } from '@/platform/assets/composables/useMediaAssetActions' -import { extractWorkflow, fetchJobDetail } from '@/platform/remote/comfyui/jobs/fetchJobs' +import { + extractWorkflow, + fetchJobDetail +} from '@/platform/remote/comfyui/jobs/fetchJobs' import { useSettingStore } from '@/platform/settings/settingStore' import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema' import { useWorkflowService } from '@/platform/workflow/core/services/workflowService' @@ -57,7 +60,7 @@ export function useJobMenu( jobId: string ): Promise => { const jobDetail = await fetchJobDetail((url) => api.fetchApi(url), jobId) - return extractWorkflow(jobDetail) + return extractWorkflow(jobDetail) as ComfyWorkflowJSON | undefined } const openJobWorkflow = async () => { diff --git a/src/platform/remote/comfyui/history/reconciliation.ts b/src/platform/remote/comfyui/history/reconciliation.ts index f4297de5b..39a8695bb 100644 --- a/src/platform/remote/comfyui/history/reconciliation.ts +++ b/src/platform/remote/comfyui/history/reconciliation.ts @@ -5,7 +5,7 @@ * Reconciles server jobs with client-cached jobs for efficient updates. * Uses job ID-based merging with create_time for sort order. */ -import type { JobListItem } from '../jobs/types/jobTypes' +import type { JobListItem } from '../jobs/jobTypes' /** * Reconciles server jobs with client-cached jobs. diff --git a/src/scripts/api.ts b/src/scripts/api.ts index 97bc3a523..c8f3cb996 100644 --- a/src/scripts/api.ts +++ b/src/scripts/api.ts @@ -45,7 +45,10 @@ import type { ComfyNodeDef } from '@/schemas/nodeDefSchema' import type { useFirebaseAuthStore } from '@/stores/firebaseAuthStore' import type { AuthHeader } from '@/types/authTypes' import type { NodeExecutionId } from '@/types/nodeIdentification' -import { fetchHistory, fetchQueue } from '@/platform/remote/comfyui/jobs/fetchJobs' +import { + fetchHistory, + fetchQueue +} from '@/platform/remote/comfyui/jobs/fetchJobs' interface QueuePromptRequestBody { client_id: string diff --git a/src/stores/queueStore.ts b/src/stores/queueStore.ts index bd31c87b6..6af3773dc 100644 --- a/src/stores/queueStore.ts +++ b/src/stores/queueStore.ts @@ -3,7 +3,10 @@ import { defineStore } from 'pinia' import { computed, ref, shallowRef, toRaw, toValue } from 'vue' import { reconcileJobs } from '@/platform/remote/comfyui/history/reconciliation' -import { extractWorkflow, fetchJobDetail } from '@/platform/remote/comfyui/jobs/fetchJobs' +import { + extractWorkflow, + fetchJobDetail +} from '@/platform/remote/comfyui/jobs/fetchJobs' import type { JobListItem } from '@/platform/remote/comfyui/jobs/jobTypes' import type { NodeId } from '@/platform/workflow/validation/schemas/workflowSchema' import type { @@ -294,7 +297,7 @@ export class TaskItemImpl { } get outputsCount(): number | undefined { - return this.job.outputs_count + return this.job.outputs_count ?? undefined } /** @@ -434,7 +437,9 @@ export class TaskItemImpl { return } - await app.loadGraphData(toRaw(workflowData)) + await app.loadGraphData( + toRaw(workflowData) as Parameters[0] + ) // Use full outputs from job detail, or fall back to existing outputs const outputsToLoad = jobDetail?.outputs ?? this.outputs diff --git a/tests-ui/tests/stores/queueStore.loadWorkflow.test.ts b/tests-ui/tests/stores/queueStore.loadWorkflow.test.ts index 028a04a8b..2b6450b81 100644 --- a/tests-ui/tests/stores/queueStore.loadWorkflow.test.ts +++ b/tests-ui/tests/stores/queueStore.loadWorkflow.test.ts @@ -1,7 +1,10 @@ import { createPinia, setActivePinia } from 'pinia' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { JobListItem } from '@/platform/remote/comfyui/jobs/jobTypes' +import type { + JobDetail, + JobListItem +} from '@/platform/remote/comfyui/jobs/jobTypes' import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema' import type { ComfyApp } from '@/scripts/app' import { TaskItemImpl } from '@/stores/queueStore' @@ -88,7 +91,7 @@ describe('TaskItemImpl.loadWorkflow - workflow fetching', () => { const task = new TaskItemImpl(job) vi.spyOn(jobsModule, 'fetchJobDetail').mockResolvedValue( - mockJobDetail as jobsModule.JobDetail + mockJobDetail as JobDetail ) await task.loadWorkflow(mockApp) @@ -117,7 +120,7 @@ describe('TaskItemImpl.loadWorkflow - workflow fetching', () => { const runningTask = new TaskItemImpl(job) vi.spyOn(jobsModule, 'fetchJobDetail').mockResolvedValue( - mockJobDetail as jobsModule.JobDetail + mockJobDetail as JobDetail ) await runningTask.loadWorkflow(mockApp)