Comfy-Org/comfy-swift-sdk

Fork 0

mirror of https://github.com/Comfy-Org/comfy-swift-sdk.git synced 2026-11-04 23:47:26 +00:00

[PR #84] feat(router): surface X-Comfy-Credits-Used on the Router run result #85

New issue

Open

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-swift-sdk/pull/84
Author: @mattmillerai
Created: 9/18/2026
Status: 🔄 Open

Base: main ← Head: matt/be-15589-router-credits-used

📝 Commits (4)

25cda28 feat(router): surface X-Comfy-Credits-Used on the run result
dd32a09 Merge remote-tracking branch 'origin/main' into matt/be-15589-router-credits-used
6aedbae fix(router): harden X-Comfy-Credits-Used read and wire it through the queue result
11fc53e fix(router): trim edge tabs from X-Comfy-Credits-Used; reject malformed spec headers

📊 Changes

9 files changed (+370 additions, -9 deletions)

View changed files

📝 .gitignore (+2 -0)
📝 README.md (+2 -0)
📝 Scripts/contract/check_router_contract.py (+89 -1)
📝 Sources/ComfySwiftSDK/Internal/RouterErrorMapping.swift (+63 -6)
📝 Sources/ComfySwiftSDK/Internal/RouterQueueTransport.swift (+2 -1)
📝 Sources/ComfySwiftSDK/Internal/RouterTransport.swift (+2 -1)
📝 Sources/ComfySwiftSDK/Public/RouterModels.swift (+30 -0)
📝 Tests/ComfySwiftSDKTests/RouterErrorMappingTests.swift (+78 -0)
📝 Tests/ComfySwiftSDKTests/RouterRunTests.swift (+102 -0)

📄 Description

ELI-5

Router tells you what a model run cost, in a response header. The Swift SDK was throwing that away, so an app had to keep its own price list to show a user what they just spent. Now the run result carries it: result.creditsUsed.

What changed

RouterRunResult.creditsUsed surfaces X-Comfy-Credits-Used, lifted in RouterErrorMapping.successMetadata right alongside the existing X-Comfy-Request-Id read so both go through the same case-insensitive header normalisation rather than a second copy of it in the transport.

Type. String?, not a parsed Double?. The wire value is a decimal at up to two places and binary floating point is the wrong type for a figure a caller reconciles against money; a caller that needs arithmetic checks the whole value is a plain decimal and then uses Decimal(string:). The check comes first because that initializer parses a prefix rather than returning nil: "1,25" reads as 1, and a duplicated header comma-joined as "1.25, 2.50" reads as 1.25.

Hygiene, and the one judgment call in the diff. The value is handed over as it arrived, trimmed — unparsed and unvalidated, because deciding that an unrecognised figure was never reported is a stronger claim than this SDK is entitled to make (so "1,25" is reported, not dropped). The two cases it does refuse are the ones where handing the value over faithfully is impossible, and both are refusals rather than repairs, because every repair invents a number: over a 32-scalar bound it is dropped rather than truncated (1000 clipped to 10 is a plausible figure that is wrong by two orders of magnitude), and a value carrying a control character, a line break or an invisible format (Cf) scalar such as a bidi override is dropped rather than scrubbed. That check runs on the value as received, before only HTTP optional whitespace is trimmed, so \n100 is refused rather than trimmed into 100 (tab is the one exception: it is optional whitespace, so it is trimmed at the edges and refused only inside the figure) — requestId's . substitution is right for an opaque id and would turn 1\n2 into an entirely credible 1.2 here. That second rule doubles as the log-injection guard for this header, since the value reaches the caller and the result's description, where it is rendered quoted and escaped so a value like 0, replayed cannot forge the replayed marker.

Replay flag on the success path. successMetadata now parses Idempotent-Replayed with the same isTrue rule as the error path, instead of taking presence as true. A proxy's false or a blank value no longer reports a charged run as a free replay, which would also explain away a missing creditsUsed.

Queued delivery. client.models queued delivery (#77) also builds RouterRunResult on its result fetch, so that path passes creditsUsed through as well.

Contract check. Scripts/contract/check_router_contract.py gains a fourth check: once the vendored spec declares a credits header on runRouterModel's 200, its name must match RouterErrorMapping.creditsUsedHeader. Until then it prints a notice and passes, the same way the queue-route check stands down.

Docs. The three caveats that actually bite are on the property and in the README: it is a price and not a settled ledger entry (the workspace's usage and invoices stay authoritative); nil means not reported and never free — the header covers a subset of providers, and a replay is not stamped with it at all, so a replayed == true response reports no figure even though the original call had a cost; and "0" is a real reported cost, so the test is presence (if let creditsUsed), never a non-zero value. The replay caveat is not in the ticket's list — it comes from the public Comfy Router API reference and limitations pages, which both state the header is optional and is not replayed.

How this was tested

swift build clean; swift test --no-parallel 378 tests / 32 suites green; Scripts/contract/check_router_contract.py in sync with the vendored spec. Six new tests: the header reading through verbatim (and into the redacted description), read case-insensitively, nil on an unstamped run, "0" staying distinguishable from absent while Decimal(value ?? "0") collapses both — which is the point of branching on presence — plus the mapping-level trim / blank / over-cap / control-character / U+2028 cases.

One hazard found reviewing my own tests and fixed before pushing: the first draft of the "0"-vs-absent test installed the stub twice with an uninstall between the two runs. TestURLProtocol.install takes a blocking mutex that only uninstall releases, so a first run that threw between them would have left it held and deadlocked every later test in the suite. Both answers now come from one scripted install.

No live Router call was made: everything is verified against TestURLProtocol stubs, which is the surface this SDK's tests have.

Residual

The other two Router disclosure headers are deliberately not in this PR. X-Comfy-Router-Fallback-Provider and X-Comfy-Router-Dropped-Params are also unsurfaced here while comfy-typescript-sdk carries both (FALLBACK_PROVIDER_HEADER / DROPPED_PARAMS_HEADER, exposed as servingProvider and droppedParams) and comfy-python-sdk carries them too. I chose to file rather than fold them in, for two reasons: Dropped-Params is a parsed list in the sibling SDKs rather than a passthrough string, and naming (servingProvider vs fallbackProvider, where the latter is already a request option) is a public-API decision worth its own review; and neither header is declared in this repo's vendored spec/router-openapi.yaml, so their doc comments would be lifted from a sibling implementation rather than from the contract. Follow-up work: surface both on RouterRunResult with the list parse and the naming settled, and reconcile the doc comments against the spec once it declares them.

The vendored spec does not declare X-Comfy-Credits-Used either, so the doc comment is not lifted from it. The contract check above now pins the name automatically once a sync declares it. spec/router-openapi.yaml at this commit (synced in #76) contains no credits header anywhere — the ask was to doc-comment the property from the vendored spec and that was not possible. The wording comes from the two public docs pages linked above plus the caveats named in the work item. When a spec sync lands the declaration, the property's doc comment and the README paragraph should be re-read against it; the contract script now catches a header name drift on that sync, but not a change in the header's documented semantics.

A charged failure still reports no cost. RouterError does not carry this header, and a partner refusal that really ran — the metered content-policy rejection the contract describes as an answer rather than a Router-level refusal — can be charged. A caller reconciling spend across failures, not just successes, still has that gap. Same five lines, on the error path; out of scope here because the work item scopes the property to the run result.

Unexercised artifacts. The upstream server-side change that ships this header, and the work item tracking the header's own coverage, are both outside this repo and could not be read from this environment; the header's name, its decimal-string-to-2dp shape, the "0"-vs-absent distinction and the not-replayed rule were all corroborated against the public docs pages instead. Coverage of the header in production (a run whose cost is known but which carries no header) is server-side and explicitly not in scope.

One claim in the work item did not hold up. It describes this SDK as lifting the least of the three and implies the Python and TypeScript SDKs already surface credits. Code search over both repos' default branches returns 0 hits for X-Comfy-Credits-Used (against 2 repos carrying the other two headers), so this is the first of the three SDKs to surface it rather than the last to catch up. Code search can lag a very recent merge, so treat that as evidence rather than proof — it changes nothing about this change, but the parity framing is worth correcting before it is repeated.

Provenance

Authored by: agent-work loop
Verified:
- Original head: swift build clean; swift test --no-parallel 378 tests in 32 suites passed.
- Current head (main merge plus two review-fix rounds): no Swift toolchain on the resolving host, so swift build / swift test were not run locally. CI's macOS Build & test job is the verification for this head; the previous head failed it only on the edge-tab credits case, which this head fixes.
- Scripts/contract/check_router_contract.py: OK on the current head (credits check stands down with a notice). Its armed path was tested against a locally patched spec, and its malformed-input path was exercised directly: headers: [] / null on the 200 now fail.
Deviations:
- The property is doc-commented from the public Comfy Router docs pages, not the vendored spec, which declares no credits header at this commit.
- The two sibling Router disclosure headers are filed as follow-up work rather than folded in (see Residual).
- An unrecognised credits shape is still forwarded verbatim. The review finding about Decimal(string:) prefix-parsing was fixed by correcting the docs, not by validating the value.

_{🔄 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/84 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/18/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `matt/be-15589-router-credits-used` --- ### 📝 Commits (4) - [`25cda28`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/25cda2821a58644b92df76bae51e08a6ca6af2c0) feat(router): surface X-Comfy-Credits-Used on the run result - [`dd32a09`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/dd32a0970665d4e9bbe164a776f6760a9760e1ed) Merge remote-tracking branch 'origin/main' into matt/be-15589-router-credits-used - [`6aedbae`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/6aedbae8367246d138b9f1d7f5ab0d7bab75a532) fix(router): harden X-Comfy-Credits-Used read and wire it through the queue result - [`11fc53e`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/11fc53e8ba92cc57ceac400621455f07c4a87294) fix(router): trim edge tabs from X-Comfy-Credits-Used; reject malformed spec headers ### 📊 Changes **9 files changed** (+370 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `.gitignore` (+2 -0) 📝 `README.md` (+2 -0) 📝 `Scripts/contract/check_router_contract.py` (+89 -1) 📝 `Sources/ComfySwiftSDK/Internal/RouterErrorMapping.swift` (+63 -6) 📝 `Sources/ComfySwiftSDK/Internal/RouterQueueTransport.swift` (+2 -1) 📝 `Sources/ComfySwiftSDK/Internal/RouterTransport.swift` (+2 -1) 📝 `Sources/ComfySwiftSDK/Public/RouterModels.swift` (+30 -0) 📝 `Tests/ComfySwiftSDKTests/RouterErrorMappingTests.swift` (+78 -0) 📝 `Tests/ComfySwiftSDKTests/RouterRunTests.swift` (+102 -0) </details> ### 📄 Description ## ELI-5 Router tells you what a model run cost, in a response header. The Swift SDK was throwing that away, so an app had to keep its own price list to show a user what they just spent. Now the run result carries it: `result.creditsUsed`. ## What changed `RouterRunResult.creditsUsed` surfaces `X-Comfy-Credits-Used`, lifted in `RouterErrorMapping.successMetadata` right alongside the existing `X-Comfy-Request-Id` read so both go through the same case-insensitive header normalisation rather than a second copy of it in the transport. **Type.** `String?`, not a parsed `Double?`. The wire value is a decimal at up to two places and binary floating point is the wrong type for a figure a caller reconciles against money; a caller that needs arithmetic checks the whole value is a plain decimal and then uses `Decimal(string:)`. The check comes first because that initializer parses a prefix rather than returning `nil`: `"1,25"` reads as `1`, and a duplicated header comma-joined as `"1.25, 2.50"` reads as `1.25`. **Hygiene, and the one judgment call in the diff.** The value is handed over as it arrived, trimmed — unparsed and unvalidated, because deciding that an unrecognised figure was *never reported* is a stronger claim than this SDK is entitled to make (so `"1,25"` is reported, not dropped). The two cases it does refuse are the ones where handing the value over faithfully is impossible, and both are refusals rather than repairs, because every repair invents a number: over a 32-scalar bound it is dropped rather than truncated (`1000` clipped to `10` is a plausible figure that is wrong by two orders of magnitude), and a value carrying a control character, a line break or an invisible format (Cf) scalar such as a bidi override is dropped rather than scrubbed. That check runs on the value as received, before only HTTP optional whitespace is trimmed, so `\n100` is refused rather than trimmed into `100` (tab is the one exception: it is optional whitespace, so it is trimmed at the edges and refused only inside the figure) — `requestId`'s `.` substitution is right for an opaque id and would turn `1\n2` into an entirely credible `1.2` here. That second rule doubles as the log-injection guard for this header, since the value reaches the caller and the result's `description`, where it is rendered quoted and escaped so a value like `0, replayed` cannot forge the `replayed` marker. **Replay flag on the success path.** `successMetadata` now parses `Idempotent-Replayed` with the same `isTrue` rule as the error path, instead of taking presence as `true`. A proxy's `false` or a blank value no longer reports a charged run as a free replay, which would also explain away a missing `creditsUsed`. **Queued delivery.** `client.models` queued delivery (#77) also builds `RouterRunResult` on its result fetch, so that path passes `creditsUsed` through as well. **Contract check.** `Scripts/contract/check_router_contract.py` gains a fourth check: once the vendored spec declares a credits header on `runRouterModel`'s `200`, its name must match `RouterErrorMapping.creditsUsedHeader`. Until then it prints a notice and passes, the same way the queue-route check stands down. **Docs.** The three caveats that actually bite are on the property and in the README: it is a price and not a settled ledger entry (the workspace's usage and invoices stay authoritative); `nil` means *not reported* and never *free* — the header covers a subset of providers, and a replay is not stamped with it at all, so a `replayed == true` response reports no figure even though the original call had a cost; and `"0"` is a real reported cost, so the test is **presence** (`if let creditsUsed`), never a non-zero value. The replay caveat is not in the ticket's list — it comes from the public Comfy Router [API reference](https://docs.comfy.org/development/comfy-router/api) and [limitations](https://docs.comfy.org/development/comfy-router/limitations) pages, which both state the header is optional and is not replayed. ## How this was tested `swift build` clean; `swift test --no-parallel` 378 tests / 32 suites green; `Scripts/contract/check_router_contract.py` in sync with the vendored spec. Six new tests: the header reading through verbatim (and into the redacted `description`), read case-insensitively, `nil` on an unstamped run, `"0"` staying distinguishable from absent while `Decimal(value ?? "0")` collapses both — which is the point of branching on presence — plus the mapping-level trim / blank / over-cap / control-character / U+2028 cases. One hazard found reviewing my own tests and fixed before pushing: the first draft of the `"0"`-vs-absent test installed the stub twice with an `uninstall` between the two runs. `TestURLProtocol.install` takes a blocking mutex that only `uninstall` releases, so a first `run` that threw between them would have left it held and deadlocked every later test in the suite. Both answers now come from one scripted install. No live Router call was made: everything is verified against `TestURLProtocol` stubs, which is the surface this SDK's tests have. ## Residual **The other two Router disclosure headers are deliberately not in this PR.** `X-Comfy-Router-Fallback-Provider` and `X-Comfy-Router-Dropped-Params` are also unsurfaced here while [comfy-typescript-sdk](https://github.com/Comfy-Org/comfy-typescript-sdk) carries both (`FALLBACK_PROVIDER_HEADER` / `DROPPED_PARAMS_HEADER`, exposed as `servingProvider` and `droppedParams`) and comfy-python-sdk carries them too. I chose to file rather than fold them in, for two reasons: `Dropped-Params` is a *parsed list* in the sibling SDKs rather than a passthrough string, and naming (`servingProvider` vs `fallbackProvider`, where the latter is already a request option) is a public-API decision worth its own review; and neither header is declared in this repo's vendored `spec/router-openapi.yaml`, so their doc comments would be lifted from a sibling implementation rather than from the contract. Follow-up work: surface both on `RouterRunResult` with the list parse and the naming settled, and reconcile the doc comments against the spec once it declares them. **The vendored spec does not declare `X-Comfy-Credits-Used` either, so the doc comment is not lifted from it.** The contract check above now pins the name automatically once a sync declares it. `spec/router-openapi.yaml` at this commit (synced in #76) contains no credits header anywhere — the ask was to doc-comment the property from the vendored spec and that was not possible. The wording comes from the two public docs pages linked above plus the caveats named in the work item. When a spec sync lands the declaration, the property's doc comment and the README paragraph should be re-read against it; the contract script now catches a header *name* drift on that sync, but not a change in the header's documented semantics. **A charged failure still reports no cost.** `RouterError` does not carry this header, and a partner refusal that really ran — the metered content-policy rejection the contract describes as an answer rather than a Router-level refusal — can be charged. A caller reconciling spend across failures, not just successes, still has that gap. Same five lines, on the error path; out of scope here because the work item scopes the property to the run *result*. **Unexercised artifacts.** The upstream server-side change that ships this header, and the work item tracking the header's own coverage, are both outside this repo and could not be read from this environment; the header's name, its decimal-string-to-2dp shape, the `"0"`-vs-absent distinction and the not-replayed rule were all corroborated against the public docs pages instead. Coverage of the header in production (a run whose cost is known but which carries no header) is server-side and explicitly not in scope. **One claim in the work item did not hold up.** It describes this SDK as lifting the least of the three and implies the Python and TypeScript SDKs already surface credits. Code search over both repos' default branches returns **0** hits for `X-Comfy-Credits-Used` (against 2 repos carrying the other two headers), so this is the *first* of the three SDKs to surface it rather than the last to catch up. Code search can lag a very recent merge, so treat that as evidence rather than proof — it changes nothing about this change, but the parity framing is worth correcting before it is repeated. ## Provenance - **Authored by:** agent-work loop - **Verified:** - Original head: `swift build` clean; `swift test --no-parallel` 378 tests in 32 suites passed. - Current head (`main` merge plus two review-fix rounds): no Swift toolchain on the resolving host, so `swift build` / `swift test` were not run locally. CI's macOS `Build & test` job is the verification for this head; the previous head failed it only on the edge-tab credits case, which this head fixes. - `Scripts/contract/check_router_contract.py`: OK on the current head (credits check stands down with a notice). Its armed path was tested against a locally patched spec, and its malformed-input path was exercised directly: `headers: []` / `null` on the `200` now fail. - **Deviations:** - The property is doc-commented from the public Comfy Router docs pages, not the vendored spec, which declares no credits header at this commit. - The two sibling Router disclosure headers are filed as follow-up work rather than folded in (see **Residual**). - An unrecognised credits shape is still forwarded verbatim. The review finding about `Decimal(string:)` prefix-parsing was fixed by correcting the docs, not by validating the value. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

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

No labels

enhancement

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Comfy-Org/comfy-swift-sdk#85

No description provided.

Rows
Columns