[PR #25] [MERGED] refactor(transport): share output-assembly tail between WS and polling paths #26

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: refactor/shared-output-assembler

📝 Commits (2)

bffadd0 refactor(transport): extract shared output-assembly tail from WS + polling paths
3c9c053 fix(transport): clean up orphaned temp files when output assembly fails partway

📊 Changes

2 files changed (+57 additions, -46 deletions)

View changed files

📝 Sources/ComfySwiftSDK/Internal/PollingFallback.swift (+52 -19)
📝 Sources/ComfySwiftSDK/Internal/WebSocketSession.swift (+5 -27)

📄 Description

ELI-5

When a workflow finishes, the SDK downloads the resulting images/videos and packs them into a WorkflowOutput. There are two ways a job can finish — over the live WebSocket, or via the polling fallback — and both did the exact same download-and-pack work with copy-pasted code. This moves that shared tail into one function both paths call. Same behavior, one copy instead of two.

What changed

Extracted the identical "download refs → WorkflowOutput" tail into a shared helper:

static func assembleOutput(
    imageRefs: [OutputFileRef],
    videoRefs: [OutputFileRef],
    transport: Transport,
    startTime: Date,
    jobId: String
) async throws -> WorkflowOutput

PollingFallback.buildOutput now calls assembleOutput after its dto-extraction + empty-refs throw.
The execution_success case in WebSocketSession.runReadLoop now calls PollingFallback.assembleOutput, passing its incrementally-buffered bufferedImageRefs / bufferedVideoRefs.

What deliberately stayed separate

The two paths collect refs differently, and that stays untouched:

WS buffers OutputFileRefs incrementally from executed frames.
buildOutput extracts them from dto.outputs.

The empty-refs handling also stays per-caller (WS yields .failed early; polling throws) — only the download-and-assemble tail is shared. ReattachCoordinator already reused buildOutput, so the WS path was the sole outlier; it now shares the same tail too.

Testing

swift build — clean.
swift test --no-parallel — 150 tests, 20 suites, all pass.

No behavior change: the extracted body is verbatim the previously-duplicated code.

Notes / judgment calls

Pure refactor, net-zero (27 insertions / 27 deletions). No public API surface changed (assembleOutput is internal).
The local Mac has only Command Line Tools (no full Xcode); the swift-testing bundle needed explicit framework/rpath flags to run locally, but that's an environment detail — CI (macos-15 + Xcode 16) runs swift test --no-parallel as normal.

_{🔄 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/25 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/4/2026 **Status:** ✅ Merged **Merged:** 7/5/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `refactor/shared-output-assembler` --- ### 📝 Commits (2) - [`bffadd0`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/bffadd0c67e05f5cde35ba1e93a5945433c0e981) refactor(transport): extract shared output-assembly tail from WS + polling paths - [`3c9c053`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/3c9c05300f465bdb13c37a8d00e1a23769d85638) fix(transport): clean up orphaned temp files when output assembly fails partway ### 📊 Changes **2 files changed** (+57 additions, -46 deletions) <details> <summary>View changed files</summary> 📝 `Sources/ComfySwiftSDK/Internal/PollingFallback.swift` (+52 -19) 📝 `Sources/ComfySwiftSDK/Internal/WebSocketSession.swift` (+5 -27) </details> ### 📄 Description ## ELI-5 When a workflow finishes, the SDK downloads the resulting images/videos and packs them into a `WorkflowOutput`. There are two ways a job can finish — over the live WebSocket, or via the polling fallback — and both did the *exact same* download-and-pack work with copy-pasted code. This moves that shared tail into one function both paths call. Same behavior, one copy instead of two. ## What changed Extracted the identical "download refs → `WorkflowOutput`" tail into a shared helper: ```swift static func assembleOutput( imageRefs: [OutputFileRef], videoRefs: [OutputFileRef], transport: Transport, startTime: Date, jobId: String ) async throws -> WorkflowOutput ``` - `PollingFallback.buildOutput` now calls `assembleOutput` after its dto-extraction + empty-refs throw. - The `execution_success` case in `WebSocketSession.runReadLoop` now calls `PollingFallback.assembleOutput`, passing its incrementally-buffered `bufferedImageRefs` / `bufferedVideoRefs`. ## What deliberately stayed separate The two paths **collect** refs differently, and that stays untouched: - WS buffers `OutputFileRef`s incrementally from `executed` frames. - `buildOutput` extracts them from `dto.outputs`. The empty-refs handling also stays per-caller (WS yields `.failed` early; polling `throw`s) — only the download-and-assemble tail is shared. `ReattachCoordinator` already reused `buildOutput`, so the WS path was the sole outlier; it now shares the same tail too. ## Testing - `swift build` — clean. - `swift test --no-parallel` — **150 tests, 20 suites, all pass.** No behavior change: the extracted body is verbatim the previously-duplicated code. ## Notes / judgment calls - Pure refactor, net-zero (27 insertions / 27 deletions). No public API surface changed (`assembleOutput` is `internal`). - The local Mac has only Command Line Tools (no full Xcode); the swift-testing bundle needed explicit framework/rpath flags to run locally, but that's an environment detail — CI (`macos-15` + Xcode 16) runs `swift test --no-parallel` as normal. --- <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:07 +00:00