[PR #43] [MERGED] refactor(job-status): decode wire job status once into a JobStatus enum #45

New issue

Closed

opened 2026-10-26 22:34:19 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:34:19 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-swift-sdk/pull/43
Author: @mattmillerai
Created: 7/11/2026
Status: ✅ Merged
Merged: 7/16/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-2857-job-status-enum

📝 Commits (2)

cb424dd refactor(job-status): decode wire status once into a JobStatus enum
34e3e3d docs(reattach): clarify only .unknown reaches the terminal-status branch

📊 Changes

5 files changed (+78 additions, -35 deletions)

View changed files

📝 Sources/ComfySwiftSDK/Internal/DTOs/JobStatusDTO.swift (+22 -1)
📝 Sources/ComfySwiftSDK/Internal/PollingFallback.swift (+12 -12)
📝 Sources/ComfySwiftSDK/Internal/ReattachCoordinator.swift (+15 -9)
📝 Tests/ComfySwiftSDKTests/OutputFetchTransientRetryTests.swift (+3 -3)
📝 Tests/ComfySwiftSDKTests/PollingFallbackTests.swift (+26 -10)

📄 Description

ELI-5

A job on the server has a status word — pending, in_progress, completed, failed, or cancelled. Our code used to take that word, lowercase it, and compare it against hand-typed strings in four different places. Type one of those strings wrong and nothing complains — it just silently falls into the "ignore it" branch.

This change decodes the word once into a real JobStatus enum. Now the four places switch on the enum instead of on strings. If someone typos a case or the server adds a new status, the compiler tells us — instead of a bug hiding in a default:.

What changed

New internal enum JobStatus: String { case pending, inProgress = "in_progress", completed, failed, cancelled, unknown } in JobStatusDTO.swift.
JobDetailResponse.status changes from String to JobStatus. A custom decoder lowercases the raw wire value and maps anything unrecognized to .unknown.
The four switch sites drop their per-use .lowercased() and switch the enum directly:
- PollingFallback.runPollLoop
- PollingFallback.derivePhase
- ReattachCoordinator.runReattach (×2)
All four switches are now exhaustive (no default:), so a new/typo'd case is a compile error.

Behavior preserved

The .unknown case does exactly what the old default: did at each site (keep polling / return "executing" / emit .queued), so unrecognized statuses are still ignored rather than treated as errors.
Decoding a missing or non-string status still throws, exactly as before — only the recognized-value mapping moved into the enum.
Status matching remains case-insensitive (the lowercasing moved from each call site into the single decoder).

Tests

Updated PollingFallbackTests / OutputFetchTransientRetryTests to use the enum (.pending, .completed, …) in assertions and DTO fixtures.
Added two decoder tests: unrecognized status → .unknown, and case-insensitive mapping ("IN_PROGRESS" → .inProgress).
Full suite green locally: 228 tests, 28 suites, all passing (swift test --no-parallel).

Judgment call

The ticket phrased the custom decoder as living "on JobDetailResponse". I put it on JobStatus itself instead — this still decodes the status exactly once and, importantly, keeps JobDetailResponse's synthesized memberwise initializer intact (the test fixtures construct JobDetailResponse(...) directly). Net decoding behavior is identical.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-swift-sdk/pull/43 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/11/2026 **Status:** ✅ Merged **Merged:** 7/16/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-2857-job-status-enum` --- ### 📝 Commits (2) - [`cb424dd`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/cb424dda87dcdf6b2b91ba3023803765400d0db6) refactor(job-status): decode wire status once into a JobStatus enum - [`34e3e3d`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/34e3e3d54b40f087db3339b70e687f70ddf9e576) docs(reattach): clarify only .unknown reaches the terminal-status branch ### 📊 Changes **5 files changed** (+78 additions, -35 deletions) <details> <summary>View changed files</summary> 📝 `Sources/ComfySwiftSDK/Internal/DTOs/JobStatusDTO.swift` (+22 -1) 📝 `Sources/ComfySwiftSDK/Internal/PollingFallback.swift` (+12 -12) 📝 `Sources/ComfySwiftSDK/Internal/ReattachCoordinator.swift` (+15 -9) 📝 `Tests/ComfySwiftSDKTests/OutputFetchTransientRetryTests.swift` (+3 -3) 📝 `Tests/ComfySwiftSDKTests/PollingFallbackTests.swift` (+26 -10) </details> ### 📄 Description ## ELI-5 A job on the server has a status word — `pending`, `in_progress`, `completed`, `failed`, or `cancelled`. Our code used to take that word, lowercase it, and compare it against hand-typed strings in **four** different places. Type one of those strings wrong and nothing complains — it just silently falls into the "ignore it" branch. This change decodes the word **once** into a real `JobStatus` enum. Now the four places switch on the enum instead of on strings. If someone typos a case or the server adds a new status, the compiler tells us — instead of a bug hiding in a `default:`. ## What changed - New internal `enum JobStatus: String { case pending, inProgress = "in_progress", completed, failed, cancelled, unknown }` in `JobStatusDTO.swift`. - `JobDetailResponse.status` changes from `String` to `JobStatus`. A custom decoder lowercases the raw wire value and maps anything unrecognized to `.unknown`. - The four switch sites drop their per-use `.lowercased()` and switch the enum directly: - `PollingFallback.runPollLoop` - `PollingFallback.derivePhase` - `ReattachCoordinator.runReattach` (×2) - All four switches are now **exhaustive** (no `default:`), so a new/typo'd case is a compile error. ## Behavior preserved - The `.unknown` case does exactly what the old `default:` did at each site (keep polling / return `"executing"` / emit `.queued`), so unrecognized statuses are still ignored rather than treated as errors. - Decoding a **missing** or non-string `status` still throws, exactly as before — only the recognized-value mapping moved into the enum. - Status matching remains case-insensitive (the lowercasing moved from each call site into the single decoder). ## Tests - Updated `PollingFallbackTests` / `OutputFetchTransientRetryTests` to use the enum (`.pending`, `.completed`, …) in assertions and DTO fixtures. - Added two decoder tests: unrecognized status → `.unknown`, and case-insensitive mapping (`"IN_PROGRESS"` → `.inProgress`). - Full suite green locally: **228 tests, 28 suites, all passing** (`swift test --no-parallel`). ## Judgment call The ticket phrased the custom decoder as living "on `JobDetailResponse`". I put it on `JobStatus` itself instead — this still decodes the status exactly once and, importantly, keeps `JobDetailResponse`'s synthesized memberwise initializer intact (the test fixtures construct `JobDetailResponse(...)` directly). Net decoding behavior is identical. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:34:19 +00:00