[PR #42] [MERGED] docs(polling): document why deriveFraction intentionally returns 0.0 (BE-2858) #44

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/42
Author: @mattmillerai
Created: 7/11/2026
Status: ✅ Merged
Merged: 7/16/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-2858-derivefraction-doc

📝 Commits (1)

7ee38ee docs(polling): document why deriveFraction intentionally returns 0.0 (BE-2858)

📊 Changes

1 file changed (+10 additions, -0 deletions)

View changed files

📝 Sources/ComfySwiftSDK/Internal/PollingFallback.swift (+10 -0)

📄 Description

ELI-5

When a job is running, the SDK likes to tell you how far along it is — a fraction from 0 to 1. That number is only ever reported by the live WebSocket progress frame. The REST "job status" endpoint the SDK polls (and reattaches to) simply doesn't include a progress number, so the helper that would compute one, deriveFraction(from:), just returns 0.0.

At a glance that looks like an unfinished stub someone forgot to fill in. It isn't — there's genuinely no field to derive a fraction from on that response. This PR adds a doc comment saying exactly that, so the next reader doesn't "fix" a 0.0 that's intentional.

What changed

Added a doc comment on PollingFallback.deriveFraction(from:) explaining that JobDetailResponse (the jobs REST DTO) carries no numeric progress signal — only status / outputs / executionError / timestamps — so 0.0 is the intentional, honest value on the polling and reattach paths. A real fraction exists only on the live WebSocket progress frame (value / max, see WebSocketSession).

No behavior change. This is a clarity-only fix.

Why not more

Per the grooming verdict on this ticket (downgraded from "implement it"), the honest minimal fix is a doc comment, not a refactor:

Don't derive it from fields — JobDetailResponse has no progress field to derive from (verified: it decodes only id, status, outputs, execution_error, create_time, update_time).
Don't delete/inline 0.0 — that's optional churn with identical behavior; keeping the named function + comment documents the intent at the call sites.

Testing

swift build — clean.
Full suite: 226 tests in 28 suites passed (swift test --no-parallel).

Notes / judgment calls

The comment notes the dead fraction != lastFraction half of runPollLoop's change-detection guard (fraction is always 0 from this source, so only the phase half ever fires) and that ReattachCoordinator emits a single 0.0 progress. Both are documented as intentional rather than changed.
Not a capability denial: the WebSocket path still delivers real fractions; this only documents the REST path's data-model reality. No negative-claim falsification obligation triggered.

_{🔄 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/42 **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-2858-derivefraction-doc` --- ### 📝 Commits (1) - [`7ee38ee`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/7ee38ee49757c555ac2f8a90bb844948fb8752de) docs(polling): document why deriveFraction intentionally returns 0.0 (BE-2858) ### 📊 Changes **1 file changed** (+10 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `Sources/ComfySwiftSDK/Internal/PollingFallback.swift` (+10 -0) </details> ### 📄 Description ## ELI-5 When a job is running, the SDK likes to tell you *how far along* it is — a fraction from 0 to 1. That number is only ever reported by the live WebSocket `progress` frame. The REST "job status" endpoint the SDK polls (and reattaches to) simply doesn't include a progress number, so the helper that would compute one, `deriveFraction(from:)`, just returns `0.0`. At a glance that looks like an unfinished stub someone forgot to fill in. It isn't — there's genuinely no field to derive a fraction from on that response. This PR adds a doc comment saying exactly that, so the next reader doesn't "fix" a `0.0` that's intentional. ## What changed - Added a doc comment on `PollingFallback.deriveFraction(from:)` explaining that `JobDetailResponse` (the jobs REST DTO) carries no numeric progress signal — only `status` / `outputs` / `executionError` / timestamps — so `0.0` is the intentional, honest value on the polling and reattach paths. A real fraction exists **only** on the live WebSocket `progress` frame (`value / max`, see `WebSocketSession`). No behavior change. This is a clarity-only fix. ## Why not more Per the grooming verdict on this ticket (downgraded from "implement it"), the honest minimal fix is a doc comment, **not** a refactor: - **Don't derive it from fields** — `JobDetailResponse` has no progress field to derive from (verified: it decodes only `id`, `status`, `outputs`, `execution_error`, `create_time`, `update_time`). - **Don't delete/inline `0.0`** — that's optional churn with identical behavior; keeping the named function + comment documents the intent at the call sites. ## Testing - `swift build` — clean. - Full suite: **226 tests in 28 suites passed** (`swift test --no-parallel`). ## Notes / judgment calls - The comment notes the dead `fraction != lastFraction` half of `runPollLoop`'s change-detection guard (fraction is always `0` from this source, so only the phase half ever fires) and that `ReattachCoordinator` emits a single `0.0` progress. Both are documented as intentional rather than changed. - Not a capability denial: the WebSocket path still delivers real fractions; this only documents the REST path's data-model reality. No negative-claim falsification obligation triggered. --- <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