Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 146 additions & 0 deletions src/platform/remote/comfyui/jobs/fetchJobs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
/**
* @fileoverview Jobs API Fetchers
* @module platform/remote/comfyui/jobs/fetchJobs
*
* Unified jobs API fetcher for history, queue, and job details.
* All distributions use the /jobs endpoint.
*/

import type { PromptId } from '@/schemas/apiSchema'

import type {
JobDetail,
JobListItem,
JobStatus,
RawJobListItem
} from './jobTypes'
import { zJobDetail, zJobsListResponse, zWorkflowContainer } from './jobTypes'

interface FetchJobsRawResult {
jobs: RawJobListItem[]
total: number
offset: number
}

/**
* Fetches raw jobs from /jobs endpoint
* @internal
*/
async function fetchJobsRaw(
fetchApi: (url: string) => Promise<Response>,
statuses: JobStatus[],
maxItems: number = 200,
offset: number = 0
): Promise<FetchJobsRawResult> {
const statusParam = statuses.join(',')
const url = `/jobs?status=${statusParam}&limit=${maxItems}&offset=${offset}`
try {
const res = await fetchApi(url)
if (!res.ok) {
console.error(`[Jobs API] Failed to fetch jobs: ${res.status}`)
return { jobs: [], total: 0, offset: 0 }
}
const data = zJobsListResponse.parse(await res.json())
return { jobs: data.jobs, total: data.pagination.total, offset }
} catch (error) {
console.error('[Jobs API] Error fetching jobs:', error)
return { jobs: [], total: 0, offset: 0 }
}
}

// Large offset to ensure running/pending jobs sort above history
const QUEUE_PRIORITY_BASE = 1_000_000

/**
* Assigns synthetic priority to jobs.
* Only assigns if job doesn't already have a server-provided priority.
*/
function assignPriority(
jobs: RawJobListItem[],
basePriority: number
): JobListItem[] {
return jobs.map((job, index) => ({
...job,
priority: job.priority ?? basePriority - index
}))
}

/**
* Fetches history (completed jobs)
* Assigns synthetic priority starting from total (lower than queue jobs).
*/
export async function fetchHistory(
fetchApi: (url: string) => Promise<Response>,
maxItems: number = 200,
offset: number = 0
): Promise<JobListItem[]> {
const { jobs, total } = await fetchJobsRaw(
fetchApi,
['completed'],
maxItems,
offset
)
// History gets priority based on total count (lower than queue)
return assignPriority(jobs, total - offset)
}

/**
* Fetches queue (in_progress + pending jobs)
* Pending jobs get highest priority, then running jobs.
*/
export async function fetchQueue(
fetchApi: (url: string) => Promise<Response>
): Promise<{ Running: JobListItem[]; Pending: JobListItem[] }> {
const { jobs } = await fetchJobsRaw(
fetchApi,
['in_progress', 'pending'],
200,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this can be lower for now? Seems like it will improve performance for very little downside. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can set it to 50. Would this affect any users since /queue didn't have a limit?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, I think it's almost unheard of to go beyond 100 in queue, the performance actually degrades a lot past that anyway.

0
)

const running = jobs.filter((j) => j.status === 'in_progress')
const pending = jobs.filter((j) => j.status === 'pending')

// Pending gets highest priority, then running
// Both are above any history job due to QUEUE_PRIORITY_BASE
return {
Running: assignPriority(running, QUEUE_PRIORITY_BASE + running.length),
Pending: assignPriority(
pending,
QUEUE_PRIORITY_BASE + running.length + pending.length
)
}
}

/**
* Fetches full job details from /jobs/{job_id}
*/
export async function fetchJobDetail(
fetchApi: (url: string) => Promise<Response>,
promptId: PromptId
): Promise<JobDetail | undefined> {
try {
const res = await fetchApi(`/jobs/${encodeURIComponent(promptId)}`)

if (!res.ok) {
console.warn(`Job not found for prompt ${promptId}`)
return undefined
}

return zJobDetail.parse(await res.json())
} catch (error) {
console.error(`Failed to fetch job detail for prompt ${promptId}:`, error)
return undefined
}
}

/**
* Extracts workflow from job detail response.
* The workflow is nested at: workflow.extra_data.extra_pnginfo.workflow
* Full workflow validation happens downstream via validateComfyWorkflow.
*/
export function extractWorkflow(job: JobDetail | undefined): unknown {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have this

export async function validateComfyWorkflow(
data: unknown,
onError: (error: string) => void = console.warn
): Promise<ComfyWorkflowJSON | null> {
const versionResult = zWorkflowVersion.safeParse(data)
let result: SafeParseReturnType<unknown, ComfyWorkflowJSON>
if (!versionResult.success) {
// Invalid workflow
const error = fromZodError(versionResult.error)
onError(`Workflow does not contain a valid version. Zod error:\n${error}`)
return null
} else if (versionResult.data.version === 1) {
// Schema version 1
result = await zComfyWorkflow1.safeParseAsync(data)
} else {
// Unknown or old version: 0.4
result = await zComfyWorkflow.safeParseAsync(data)
}
if (result.success) return result.data
const error = fromZodError(result.error)
onError(`Invalid workflow against zod schema:\n${error}`)
return null
}

and this

Which covers different workflow schema versions and some other things.

const parsed = zWorkflowContainer.safeParse(job?.workflow)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validation was traditionally set as an option, due to the performance overhead and some users annoyed by constant warnings of invalid workflows that still technically worked (they don't care if it's invalid)

{
id: 'Comfy.Validation.Workflows',
name: 'Validate workflows',
type: 'boolean',
defaultValue: isCloud ? false : true
},

if (!parsed.success) return undefined
return parsed.data.extra_data?.extra_pnginfo?.workflow
}
107 changes: 107 additions & 0 deletions src/platform/remote/comfyui/jobs/jobTypes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/**
* @fileoverview Jobs API types - Backend job API format
* @module platform/remote/comfyui/jobs/jobTypes
*
* These types represent the jobs API format returned by the backend.
* Jobs API provides a memory-optimized alternative to history API.
*/

import { z } from 'zod'

import { resultItemType, zTaskOutput } from '@/schemas/apiSchema'

const zJobStatus = z.enum([
'pending',
'in_progress',
'completed',
'failed',
'cancelled'
])

const zPreviewOutput = z.object({
filename: z.string(),
subfolder: z.string(),
type: resultItemType
})

/**
* Execution error details for error jobs.
* Contains the same structure as ExecutionErrorWsMessage from WebSocket.
*/
const zExecutionError = z
.object({
prompt_id: z.string().optional(),
timestamp: z.number().optional(),
node_id: z.string(),
node_type: z.string(),
executed: z.array(z.string()).optional(),
exception_message: z.string(),
exception_type: z.string(),
traceback: z.array(z.string()),
current_inputs: z.unknown(),
current_outputs: z.unknown()
Comment on lines +41 to +42
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to have the types tighter as the contract stabilizes.

})
.passthrough()

/**
* Raw job from API - uses passthrough to allow extra fields
*/
const zRawJobListItem = z
.object({
id: z.string(),
status: zJobStatus,
create_time: z.number(),
execution_start_time: z.number().nullable().optional(),
execution_end_time: z.number().nullable().optional(),
Comment on lines +54 to +55
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both nullable and optional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're still nullable and optional. Why not just optional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to push changes. execution_start_time = null seems more correct for jobs that haven't executed yet rather than being omitted wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I'm in the Never-null camp, especially over the wire. But it's your API.

preview_output: zPreviewOutput.nullable().optional(),
outputs_count: z.number().nullable().optional(),
execution_error: zExecutionError.nullable().optional(),
workflow_id: z.string().nullable().optional(),
priority: z.number().optional()
})
.passthrough()

/**
* Job detail - returned by GET /api/jobs/{job_id} (detail endpoint)
* Includes full workflow and outputs for re-execution and downloads
*/
export const zJobDetail = zRawJobListItem
.extend({
workflow: z.unknown().optional(),
outputs: zTaskOutput.optional(),
update_time: z.number().optional(),
execution_status: z.unknown().optional(),
execution_meta: z.unknown().optional()
})
.passthrough()
Comment on lines +68 to +76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Job detail schema appropriately extends the base schema.

The use of z.unknown() for workflow, execution_status, and execution_meta is acceptable for infrastructure code requiring flexibility. The PR mentions an extractWorkflow utility will handle workflow data extraction.

Future refinement: Consider defining a specific schema for workflow once its structure stabilizes, even if it uses .passthrough() for extensibility:

const zWorkflowStructure = z.object({
  // Known workflow fields
  nodes: z.array(z.unknown()).optional(),
  // ... other known fields
}).passthrough()

// Then use:
workflow: zWorkflowStructure.optional(),
🤖 Prompt for AI Agents
In src/platform/remote/comfyui/jobs/jobTypes.ts around lines 68 to 76, replace
the generic z.unknown() for the workflow field with a named, extensible workflow
schema: create a const zWorkflowStructure = z.object({ nodes:
z.array(z.unknown()).optional() /* add other known fields here as they stabilize
*/ }).passthrough() and then change workflow: z.unknown().optional() to
workflow: zWorkflowStructure.optional(); leave execution_status and
execution_meta as z.unknown().optional() for now.


const zPaginationInfo = z.object({
offset: z.number(),
limit: z.number(),
total: z.number(),
has_more: z.boolean()
})

export const zJobsListResponse = z.object({
jobs: z.array(zRawJobListItem),
pagination: zPaginationInfo
})

/** Schema for workflow container structure in job detail responses */
export const zWorkflowContainer = z.object({
extra_data: z
.object({
extra_pnginfo: z
.object({
workflow: z.unknown()
})
.optional()
})
.optional()
})

export type JobStatus = z.infer<typeof zJobStatus>
export type RawJobListItem = z.infer<typeof zRawJobListItem>
/** Job list item with priority always set (server-provided or synthetic) */
export type JobListItem = RawJobListItem & { priority: number }
export type JobDetail = z.infer<typeof zJobDetail>
Comment on lines +1 to +107
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider leveraging Zod 4 features for enhanced validation.

The schemas are well-designed for forward compatibility. As this API matures, consider using Zod 4's standalone string format validators for ID fields if they follow specific patterns.

Based on library documentation, Zod 4 introduced standalone validators like:

  • z.uuid() for strict RFC-compliant UUIDs
  • z.ulid() for ULIDs
  • z.cuid2() for CUIDs
  • z.nanoid() for NanoIDs

Example if job IDs are UUIDs:

 const zRawJobListItem = z
   .object({
-    id: z.string(),
+    id: z.uuid(),
     status: zJobStatus,
     // ...

This would provide stricter validation and better error messages at runtime.

🤖 Prompt for AI Agents
In src/platform/remote/comfyui/jobs/jobTypes.ts lines 1-107, several ID fields
are currently defined as z.string(); update those to use stricter Zod validators
(e.g., z.string().uuid() or, if you know the ID format,
z.string().ulid()/z.string().cuid2()/z.string().nanoid()) to enforce correct ID
formats at runtime: replace id: z.string() with id: z.string().uuid(),
prompt_id: z.string().optional() -> prompt_id: z.string().uuid().optional(),
node_id: z.string() -> node_id: z.string().uuid(), and workflow_id:
z.string().nullable().optional() -> workflow_id:
z.string().uuid().nullable().optional(); keep existing optional/nullable
chaining and passthroughs unchanged, and pick the appropriate validator if IDs
are not UUIDs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to upgrade to Zod 4 first.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the current Zod version in package.json files
echo "=== Checking Zod version in package.json ==="
if [ -f package.json ]; then
  cat package.json | jq -r '.dependencies.zod // .devDependencies.zod // "not found in main package.json"'
else
  echo "package.json not found"
fi

echo ""
echo "=== Checking for other package.json files ==="
fd -t f "^package\.json$" -x sh -c 'echo "File: {}" && cat {} | jq -r ".dependencies.zod // .devDependencies.zod // empty" | sed "s/^/  Version: /"'

Length of output: 364


🏁 Script executed:

#!/bin/bash
# Check for catalog definition in workspace configuration
echo "=== Checking for catalog definition ==="

# Check pnpm-workspace.yaml
if [ -f pnpm-workspace.yaml ]; then
  echo "Found pnpm-workspace.yaml:"
  cat pnpm-workspace.yaml
fi

# Check for catalog definition in package.json
if [ -f package.json ]; then
  echo ""
  echo "=== Checking catalog in package.json ==="
  cat package.json | jq '.catalog // .pnpm.catalog // empty'
fi

# Check pnpm-lock.yaml for actual zod version
if [ -f pnpm-lock.yaml ]; then
  echo ""
  echo "=== Checking Zod version in pnpm-lock.yaml ==="
  grep -A 2 "zod@" pnpm-lock.yaml | head -20
fi

Length of output: 3929


@DrJKL You're absolutely right! I can see the project is currently using Zod ^3.23.8 in the catalog (with 3.24.1 installed). The standalone format validators like z.uuid() are Zod 4 features, so this enhancement should be revisited after the upgrade.

Would you like me to create a tracking issue to revisit stricter ID validation once Zod 4 is available? This way, the suggestion won't be lost when you do upgrade.


✏️ Learnings added
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: The ComfyUI_frontend project currently uses Zod 3 (^3.23.8 in catalog). Zod 4-specific features like standalone format validators (z.uuid(), z.ulid(), z.cuid2(), z.nanoid()) are not available and should not be suggested until after the project upgrades to Zod 4.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, go for it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Create a GitHub issue to track the Zod 4 ID validation enhancement

gh issue create \
  --title "Enhance ID validation using Zod 4 format validators" \
  --body "After upgrading to Zod 4, revisit ID field validation in the Jobs API types to use stricter format validators.

**Context:**
The Jobs API types in \`src/platform/remote/comfyui/jobs/jobTypes.ts\` currently use \`z.string()\` for ID fields. Zod 4 introduces standalone format validators (\`z.uuid()\`, \`z.ulid()\`, \`z.cuid2()\`, \`z.nanoid()\`) that would provide stricter runtime validation and better error messages.

**Affected fields:**
- \`id\` in \`zRawJobListItem\`
- \`prompt_id\` in \`zExecutionError\`
- \`node_id\` in \`zExecutionError\`
- \`workflow_id\` in \`zRawJobListItem\`

**Prerequisite:**
This enhancement requires upgrading from Zod 3 (currently ^3.23.8) to Zod 4.

**Related:**
- PR: https://github.com/Comfy-Org/ComfyUI_frontend/pull/7169
- Discussion: https://github.com/Comfy-Org/ComfyUI_frontend/pull/7169#discussion_r2600426728
- Requested by: @DrJKL" \
  --assignee DrJKL

Length of output: 57


@DrJKL Done! I've created issue #7273 to track the ID validation enhancement after the Zod 4 upgrade. The issue is assigned to you and includes all the relevant context and backlinks.


🧠 Learnings used
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: The ComfyUI_frontend project currently uses Zod 3 (^3.23.8 in catalog). Zod 4-specific features like standalone format validators (z.uuid(), z.ulid(), z.cuid2(), z.nanoid()) are not available and should not be suggested until after the project upgrades to Zod 4.

Loading