[PR #210] feat(models): expose the queued-submit cost estimate as RequestHandle.estimate #210

New issue

Open

opened 2026-10-31 22:05:43 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 22:05:43 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-typescript-sdk/pull/210
Author: @mattmillerai
Created: 10/9/2026
Status: 🔄 Open

Base: main ← Head: be-19976-request-handle-estimate

📝 Commits (2)

35c2461 feat(models): expose the queued-submit cost estimate as RequestHandle.estimate
7630bb8 fix(models): reject a quote missing its figure or with a non-numeric dollar string

📊 Changes

6 files changed (+454 additions, -4 deletions)

View changed files

📝 CHANGELOG.md (+12 -0)
📝 README.md (+10 -0)
📝 src/sdk/index.ts (+1 -0)
📝 src/sdk/modelRequests.test.ts (+206 -2)
📝 src/sdk/modelRequests.ts (+221 -2)
📝 src/sdk/models.ts (+4 -0)

📄 Description

ELI-5

When you queue a model request, Router can send back a price quote with the acceptance. Until now the SDK threw that quote away. Now it is on the handle you get back, as handle.estimate. It is null when there is no quote, and null never means "free".

What changed

CostEstimate (exported type): source is the server's open string, and isExact / isEstimated / isUnknown are the booleans to branch on. A source this release does not recognise reads as unknown, which is what the contract asks for. It also carries currency, provider, model and pricingAsOf. The dollar figures amount / minAmount / maxAmount are decimal strings, so nothing gets rounded. amountCents / minAmountCents / maxAmountCents / credits are numbers, and reason and raw are included too. An optional field the server leaves out reads as null, following the repo's QueueUpdate style.
costEstimateOf(value) in src/sdk/modelRequests.ts is a lenient reader that never throws. Any malformed shape gives null: not an object, a required field missing or empty, an optional field present with the wrong type or a non-finite number, a dollar figure that is not a plain decimal numeral, an exact quote with no amount, or an estimated quote missing a bound. source is compared verbatim (not trimmed), and raw is a frozen shallow copy of the wire object. A malformed quote never fails a submit the server has already accepted.
RequestHandle.estimate: CostEstimate | null is set from the submit body in models.submit. subscribe builds its handle internally and does not expose it. It is null on a handle rebuilt by comfy.models.handle, and null on an idempotent replay: if Idempotent-Replayed is present the estimate is forced to null even when the body carries one, because the server does not store the original quote.
Test harness: queueScript gets an estimate knob (left out by default) and submitHeaders. Tests cover exact, estimated, unknown, unrecognised and untrimmed source, absent, 24 malformed shapes (including missing figures and non-numeric dollar strings), non-finite numbers, the frozen raw copy, replay and a rebuilt handle.
README section, CHANGELOG Unreleased › Added entry, and a TSDoc note on Models.submit.

Judgment calls

Strict on a field with the wrong type. If any field is present with the wrong type, the whole quote is null. Dropping only that field would leave a partial quote, e.g. isExact with amount: null, which a caller could misread as zero. The contract says an absent amount must never be read as free.
The figure the source promises is required. An exact quote with no amount, or an estimated quote missing a bound, reads as null. The contract says exact carries amount and estimated carries both bounds. Returning one without them would give a caller isExact with amount: null, the misread-as-zero case this reader exists to prevent. (Changed after review; the first version checked each field on its own.)
reason stays verbatim. The contract reads any reason other than not_quotable / unpriced_model as not_quotable. The TSDoc says so and suggests branching on unpriced_model, rather than adding a derived flag the Python twin doesn't have.
costEstimateOf is not re-exported from the package root. Only the CostEstimate type is. The reader is reachable from the module for tests and can be promoted if a models.estimate() method lands.

Provenance

Authored by: agent-work loop
Verified:
- Tests: 800 passed, 5 skipped (pnpm test), including the RequestHandle.estimate cases
- Lint / format / typecheck clean (pnpm lint, pnpm format:check, pnpm typecheck); pnpm check:spec-drift OK; pnpm build OK
- Replay guard checked: the replay test sends a valid estimate with Idempotent-Replayed: true, so dropping the guard would fail it
- Public-repo hygiene: the shared workflow passed on the previous head; I grepped the added lines for ticket-shaped ids and internal links and found none
- CI sdk-parity is red on this PR and on main, and the cause is not in this PR: the Python SDK added QueueBacklogFull, which makes the snapshot stale. #209 refreshes the snapshot and retires that lead
Deviations: the cross-SDK name check against the Python twin was not done, because no Python counterpart exists yet (see Residual)

Residual

Python parity, not done. The plan asks to re-run the sdk-parity check against the Python SDK's matching change so both export the same public name (CostEstimate). No such Python PR existed when this was written. A search of open and closed Python SDK PRs for "estimate" found only spec syncs. Two follow-ups: (1) land the Python twin (CostEstimate with is_exact / is_estimated / is_unknown, and RequestHandle.estimate), and (2) decide whether scripts/python-surface.mjs / src/sdk/surface-parity.test.ts should start diffing data-type names like CostEstimate. Today they compare only models methods and error classes, so a name mismatch here would not fail any check.
The vendored spec on main does not declare estimate on the submit yet. RouterQueueSubmitFields.estimate comes in with the still-open Router spec sync PR (#209). RouterCostEstimate itself is already on main, and this change reads the field the way #209 declares it. Nothing is generated from the Router spec, and no contract test pins this field, so the two PRs can merge in either order. After #209 lands, someone could add a contract assertion that RouterQueueSubmitFields declares estimate → RouterCostEstimate.
Not exercised against a live Router. Verified only against the stub server. The queued routes are gated server-side, and the estimate is enabled per caller.
reason values byok and pricing_unavailable come in with #209. They are passed through as the server's open string. The docs list only not_quotable / unpriced_model, the values main's spec documents today.

Summary by CodeRabbit

New Features
- Request handles now expose an optional cost estimate, including its pricing classification and monetary values. The CostEstimate type is also available for use in TypeScript.
- Unavailable or invalid quotes are reported as null; this does not indicate that a request is free.
Documentation
- Added guidance on estimate types, pricing values, and when quotes may be unavailable.

_{🔄 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-typescript-sdk/pull/210 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 10/9/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `be-19976-request-handle-estimate` --- ### 📝 Commits (2) - [`35c2461`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/35c246105845265ac7f50b5adfdc3615cc423540) feat(models): expose the queued-submit cost estimate as RequestHandle.estimate - [`7630bb8`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/7630bb819de367a4ff86fe3c06b29fa8a6456553) fix(models): reject a quote missing its figure or with a non-numeric dollar string ### 📊 Changes **6 files changed** (+454 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+12 -0) 📝 `README.md` (+10 -0) 📝 `src/sdk/index.ts` (+1 -0) 📝 `src/sdk/modelRequests.test.ts` (+206 -2) 📝 `src/sdk/modelRequests.ts` (+221 -2) 📝 `src/sdk/models.ts` (+4 -0) </details> ### 📄 Description ## ELI-5 When you queue a model request, Router can send back a price quote with the acceptance. Until now the SDK threw that quote away. Now it is on the handle you get back, as `handle.estimate`. It is `null` when there is no quote, and `null` never means "free". ## What changed - **`CostEstimate`** (exported type): `source` is the server's open string, and `isExact` / `isEstimated` / `isUnknown` are the booleans to branch on. A source this release does not recognise reads as unknown, which is what the contract asks for. It also carries `currency`, `provider`, `model` and `pricingAsOf`. The dollar figures `amount` / `minAmount` / `maxAmount` are **decimal strings**, so nothing gets rounded. `amountCents` / `minAmountCents` / `maxAmountCents` / `credits` are numbers, and `reason` and `raw` are included too. An optional field the server leaves out reads as `null`, following the repo's `QueueUpdate` style. - **`costEstimateOf(value)`** in `src/sdk/modelRequests.ts` is a lenient reader that never throws. Any malformed shape gives `null`: not an object, a required field missing or empty, an optional field present with the wrong type or a non-finite number, a dollar figure that is not a plain decimal numeral, an `exact` quote with no `amount`, or an `estimated` quote missing a bound. `source` is compared verbatim (not trimmed), and `raw` is a frozen shallow copy of the wire object. A malformed quote never fails a submit the server has already accepted. - **`RequestHandle.estimate: CostEstimate | null`** is set from the submit body in `models.submit`. `subscribe` builds its handle internally and does not expose it. It is `null` on a handle rebuilt by `comfy.models.handle`, and `null` on an idempotent replay: if `Idempotent-Replayed` is present the estimate is forced to `null` even when the body carries one, because the server does not store the original quote. - Test harness: `queueScript` gets an `estimate` knob (left out by default) and `submitHeaders`. Tests cover exact, estimated, unknown, unrecognised and untrimmed source, absent, 24 malformed shapes (including missing figures and non-numeric dollar strings), non-finite numbers, the frozen `raw` copy, replay and a rebuilt handle. - README section, CHANGELOG `Unreleased › Added` entry, and a TSDoc note on `Models.submit`. ## Judgment calls - **Strict on a field with the wrong type.** If any field is present with the wrong type, the whole quote is `null`. Dropping only that field would leave a partial quote, e.g. `isExact` with `amount: null`, which a caller could misread as zero. The contract says an absent amount must never be read as free. - **The figure the source promises is required.** An `exact` quote with no `amount`, or an `estimated` quote missing a bound, reads as `null`. The contract says `exact` carries `amount` and `estimated` carries both bounds. Returning one without them would give a caller `isExact` with `amount: null`, the misread-as-zero case this reader exists to prevent. (Changed after review; the first version checked each field on its own.) - **`reason` stays verbatim.** The contract reads any `reason` other than `not_quotable` / `unpriced_model` as `not_quotable`. The TSDoc says so and suggests branching on `unpriced_model`, rather than adding a derived flag the Python twin doesn't have. - **`costEstimateOf` is not re-exported from the package root.** Only the `CostEstimate` type is. The reader is reachable from the module for tests and can be promoted if a `models.estimate()` method lands. ## Provenance - **Authored by:** agent-work loop - **Verified:** - Tests: 800 passed, 5 skipped (`pnpm test`), including the `RequestHandle.estimate` cases - Lint / format / typecheck clean (`pnpm lint`, `pnpm format:check`, `pnpm typecheck`); `pnpm check:spec-drift` OK; `pnpm build` OK - Replay guard checked: the replay test sends a valid estimate with `Idempotent-Replayed: true`, so dropping the guard would fail it - Public-repo hygiene: the shared workflow passed on the previous head; I grepped the added lines for ticket-shaped ids and internal links and found none - CI `sdk-parity` is red on this PR and on `main`, and the cause is not in this PR: the Python SDK added `QueueBacklogFull`, which makes the snapshot stale. #209 refreshes the snapshot and retires that lead - **Deviations:** the cross-SDK name check against the Python twin was not done, because no Python counterpart exists yet (see Residual) ## Residual - **Python parity, not done.** The plan asks to re-run the sdk-parity check against the Python SDK's matching change so both export the same public name (`CostEstimate`). No such Python PR existed when this was written. A search of open and closed Python SDK PRs for "estimate" found only spec syncs. Two follow-ups: (1) land the Python twin (`CostEstimate` with `is_exact` / `is_estimated` / `is_unknown`, and `RequestHandle.estimate`), and (2) decide whether `scripts/python-surface.mjs` / `src/sdk/surface-parity.test.ts` should start diffing data-type names like `CostEstimate`. Today they compare only `models` methods and error classes, so a name mismatch here would not fail any check. - **The vendored spec on `main` does not declare `estimate` on the submit yet.** `RouterQueueSubmitFields.estimate` comes in with the still-open Router spec sync PR (#209). `RouterCostEstimate` itself is already on `main`, and this change reads the field the way #209 declares it. Nothing is generated from the Router spec, and no contract test pins this field, so the two PRs can merge in either order. After #209 lands, someone could add a contract assertion that `RouterQueueSubmitFields` declares `estimate` → `RouterCostEstimate`. - **Not exercised against a live Router.** Verified only against the stub server. The queued routes are gated server-side, and the estimate is enabled per caller. - **`reason` values `byok` and `pricing_unavailable`** come in with #209. They are passed through as the server's open string. The docs list only `not_quotable` / `unpriced_model`, the values `main`'s spec documents today.  ## Summary by CodeRabbit * **New Features** * Request handles now expose an optional cost estimate, including its pricing classification and monetary values. The `CostEstimate` type is also available for use in TypeScript. * Unavailable or invalid quotes are reported as `null`; this does not indicate that a request is free. * **Documentation** * Added guidance on estimate types, pricing values, and when quotes may be unavailable.  --- <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-31 22:05:43 +00:00