[PR #5] [MERGED] test + fix: SSE/core/translate coverage, and monotonic progress across reconnect #15

Closed
opened 2026-10-31 22:03:50 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-typescript-sdk/pull/5
Author: @wei-hai
Created: 7/20/2026
Status: ✅ Merged
Merged: 7/20/2026
Merged by: @wei-hai

Base: main ← Head: test/comprehensive-qa


📝 Commits (2)

  • b34757f test: cover the SSE decoder, sans-IO core walk, translate, and poll backstop
  • 3442242 fix: guarantee monotonic progress across an SSE reconnect

📊 Changes

6 files changed (+219 additions, -2 deletions)

View changed files

➕ src/low/sse.test.ts (+64 -0)
➕ src/sdk/core.test.ts (+90 -0)
📝 src/sdk/exceptions.test.ts (+20 -0)
📝 src/sdk/jobs.test.ts (+28 -0)
📝 src/sdk/jobs.ts (+9 -0)
📝 test/support/stub-server.ts (+8 -2)

📄 Description

Adds 16 tests for logic the existing suite only exercised incidentally. Two source files with real logic (src/low/sse.ts, src/sdk/core.ts) had no dedicated test file and were only reached through jobs/assets tests, leaving their edge branches dark.

What's added

  • src/low/sse.test.ts (new) — iterateSse frame parsing: multi-line data: joined with newlines, comment/keepalive lines skipped, a data-less frame dropped, a frame reassembled across chunk boundaries (including a split mid-UTF-8 character), and the non-object / non-JSON payload fallbacks ({value} / {raw}).
  • src/sdk/core.test.ts (new) — findAssetHandles / substituteAssetHandles walking arrays and nested objects (batch inputs), the "handle not materialized" throw, plus assetReference field selection, isTerminal (canceling is deliberately not terminal), and backoffSchedule growth/cap.
  • src/sdk/exceptions.test.ts — translate() re-raises an ApiError as its idiomatic SDK exception and passes a non-ApiError (e.g. a network TypeError) through by identity, never wrapping it.
  • src/sdk/jobs.test.ts — events() settles from the poll backstop with no second SSE connection when a mid-stream drop leaves the job already terminal (the largest previously-uncovered block).

Coverage: 90.8% → 92.9% statements, 81.9% → 84.7% branches (sse.ts 73→91, core.ts 80→98, jobs.ts 86→91). Full suite 104 passed; tsc, lint, format, spec-drift all clean.

Fixed in this PR

  • Monotonic progress across a reconnect. Job.events() now suppresses a progress frame whose value regresses below the highest already emitted, so a consumer'''s progress never goes backwards; the guard persists across reconnects. Regression-tested.

Still flagged (needs a spec change, not fixable in the SDK)

  • 202 scan-pending. The SDK can'''t poll to readiness because the contract has no observable signal: getAsset documents only 200 (no 202) and the Asset schema has no scan/ready field. Defining that readiness signal is a cloud-spec change first.

🔄 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/5 **Author:** [@wei-hai](https://github.com/wei-hai) **Created:** 7/20/2026 **Status:** ✅ Merged **Merged:** 7/20/2026 **Merged by:** [@wei-hai](https://github.com/wei-hai) **Base:** `main` ← **Head:** `test/comprehensive-qa` --- ### 📝 Commits (2) - [`b34757f`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/b34757fab05b14b1606bbda5e8888273b6a8c101) test: cover the SSE decoder, sans-IO core walk, translate, and poll backstop - [`3442242`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/3442242e2f67a79b3a9ce5a17350b708565822d8) fix: guarantee monotonic progress across an SSE reconnect ### 📊 Changes **6 files changed** (+219 additions, -2 deletions) <details> <summary>View changed files</summary> ➕ `src/low/sse.test.ts` (+64 -0) ➕ `src/sdk/core.test.ts` (+90 -0) 📝 `src/sdk/exceptions.test.ts` (+20 -0) 📝 `src/sdk/jobs.test.ts` (+28 -0) 📝 `src/sdk/jobs.ts` (+9 -0) 📝 `test/support/stub-server.ts` (+8 -2) </details> ### 📄 Description Adds 16 tests for logic the existing suite only exercised incidentally. Two source files with real logic (`src/low/sse.ts`, `src/sdk/core.ts`) had no dedicated test file and were only reached through `jobs`/`assets` tests, leaving their edge branches dark. ## What's added - **`src/low/sse.test.ts`** (new) — `iterateSse` frame parsing: multi-line `data:` joined with newlines, comment/keepalive lines skipped, a data-less frame dropped, a frame reassembled across chunk boundaries (including a split mid-UTF-8 character), and the non-object / non-JSON payload fallbacks (`{value}` / `{raw}`). - **`src/sdk/core.test.ts`** (new) — `findAssetHandles` / `substituteAssetHandles` walking arrays and nested objects (batch inputs), the "handle not materialized" throw, plus `assetReference` field selection, `isTerminal` (`canceling` is deliberately not terminal), and `backoffSchedule` growth/cap. - **`src/sdk/exceptions.test.ts`** — `translate()` re-raises an `ApiError` as its idiomatic SDK exception and passes a non-`ApiError` (e.g. a network `TypeError`) through by identity, never wrapping it. - **`src/sdk/jobs.test.ts`** — `events()` settles from the poll backstop with **no second SSE connection** when a mid-stream drop leaves the job already terminal (the largest previously-uncovered block). Coverage: **90.8% → 92.9%** statements, **81.9% → 84.7%** branches (`sse.ts` 73→91, `core.ts` 80→98, `jobs.ts` 86→91). Full suite **104 passed**; tsc, lint, format, spec-drift all clean. ## Fixed in this PR - **Monotonic progress across a reconnect.** `Job.events()` now suppresses a `progress` frame whose value regresses below the highest already emitted, so a consumer'''s progress never goes backwards; the guard persists across reconnects. Regression-tested. ## Still flagged (needs a spec change, not fixable in the SDK) - **`202` scan-pending.** The SDK can'''t poll to readiness because the contract has no observable signal: `getAsset` documents only `200` (no `202`) and the `Asset` schema has no scan/ready field. Defining that readiness signal is a cloud-spec change first. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 22:03:50 +00:00
Sign in to join this conversation.
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-typescript-sdk#15
No description provided.