mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-23 00:04:06 +00:00
chore: Remove migration plan doc
This commit is contained in:
@@ -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
|
||||
```
|
||||
@@ -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<ComfyWorkflowJSON | undefined> => {
|
||||
const jobDetail = await fetchJobDetail((url) => api.fetchApi(url), jobId)
|
||||
return extractWorkflow(jobDetail)
|
||||
return extractWorkflow(jobDetail) as ComfyWorkflowJSON | undefined
|
||||
}
|
||||
|
||||
const openJobWorkflow = async () => {
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<typeof app.loadGraphData>[0]
|
||||
)
|
||||
|
||||
// Use full outputs from job detail, or fall back to existing outputs
|
||||
const outputsToLoad = jobDetail?.outputs ?? this.outputs
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user