Skip to content

refactor(sse): narrow three media-generation result unions - #8645

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.49from
backryun:chore/ts7-types-media-result-unions
Jul 27, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.49from
backryun:chore/ts7-types-media-result-unions

Conversation

@backryun

Copy link
Copy Markdown
Contributor

Part of #8484. Seven diagnostics across five files, one cause — and the three unions get three different treatments, which is the interesting part.

The cause, again

strictNullChecks: false means ok: true | false narrows the positive branch but leaves the negative one as the whole union. So if (!result.ok) cannot reach status/error. Fifth time this campaign (#8483, #8499, #8531, #8638).

Three unions, three treatments

union visibility treatment fixed
SegmindRequestResult exported type predicate 4 (image + video providers)
downloadGeneratedImage's return module-private retag 2 (freepik)
ValidatedMediaGenerationBody module-private retag 1 (videos route)

Per the rule recorded on #8499 — predicate when exported, retag when private. SegmindRequestResult is the published return contract of segmindRequest(), so its ok shape stays put and gets:

export function isSegmindFailure(
  result: SegmindRequestResult
): result is Extract<SegmindRequestResult, { ok: false }> {
  return !result.ok;
}

The other two have no consumer outside their module, so retagging is cheaper and clearer. Both consumers of the retagged body union — the videos and music routes — were updated with it.

What I tried, measured, and reverted

MediaGenerationResult in the same shared file looks like the same shape:

type MediaGenerationResult =
  | { success: true; data: unknown }
  | { success: false; error: unknown; status: number };

Retagging it and narrowing failedMediaGenerationResponse's parameter to the failed arm fixes 2 more — but introduces one new error: the provider handlers that produce these results widen success to boolean rather than a literal, so the route's result no longer matches the narrowed parameter.

That is a different root cause — literal widening at the producers, not discriminant narrowing at the consumer — and fixing it properly means going after the producers first. Measured 10 fixed / 1 new, reverted that part, kept the invariant. Left for its own slice.

Verification

208 → 201, zero new errors, on a line-number-agnostic diff of the full tsc error set.

  • npm run typecheck:core — clean
  • eslint on all seven changed files — clean
  • check:file-size — clean
  • 234/234 across the 27 affected suites

No new tests — the failure arms are exercised

Checking the arms the change touches is standard here, and both retagged/predicated failure paths already run under test:

  • segmind-image-video-provider-6656.test.ts drives a 500 (text/plain body) and a 502 through both segmind providers — the exact ok: false path isSegmindFailure now guards
  • video-generation-handler.test.ts covers a 503 and a malformed-output 500

The freepik download failure and the invalid-JSON body arm are narrower; they are reachable and unchanged in behaviour (retagging a discriminant has no runtime effect beyond the literal string compared), so I did not manufacture coverage for them.

Seven diagnostics across five files, one cause — the `strictNullChecks: false`
limitation again: `ok: true | false` narrows the positive branch but leaves the
negative one as the whole union, so `if (!result.ok)` cannot reach
`status`/`error`.

Three unions, and the treatment differs per the rule recorded on diegosouzapw#8499:

  SegmindRequestResult          exported  -> type predicate  (4: image + video)
  downloadGeneratedImage        private   -> retag            (2: freepik)
  ValidatedMediaGenerationBody  private   -> retag            (1: videos route)

`SegmindRequestResult` gets `isSegmindFailure()` rather than a retag because it
is exported and its `ok` shape is the published contract of `segmindRequest()`;
the other two are module-local with no external consumer, so retagging is the
cheaper fix. Both consumers of the retagged body union (videos + music routes)
updated with it.

208 -> 201, zero new, on a line-number-agnostic diff of the full tsc error set.

Deliberately excluded: `MediaGenerationResult` and its `failedMediaGenerationResponse`
parameter, which look like the same shape. Retagging it fixes 2 more but breaks
the callers, because the provider handlers that produce these results widen
`success` to `boolean` rather than a literal — a different root cause (literal
widening, not discriminant narrowing) that needs the producers fixed first. I
tried it, measured 10 fixed / 1 new, and reverted that part to keep the
zero-new-errors invariant. Left for its own slice.

No new tests: the failure arms are already exercised —
segmind-image-video-provider-6656.test.ts drives 500 and 502 upstream responses
through both segmind providers, and video-generation-handler.test.ts covers 503
and a malformed-output 500. 234/234 across the 27 segmind / freepik / video /
music / image / media suites; typecheck:core, eslint and check:file-size clean.
@backryun
backryun force-pushed the chore/ts7-types-media-result-unions branch from 73c37ff to 83554fd Compare July 26, 2026 20:53
@diegosouzapw

Copy link
Copy Markdown
Owner

Validated in local merge-train /tmp/train1d-20260727-090022-suite.log on .113 @ 029cdf4215cf465f0e1716ac9f84a84692b1e881 (full unit suite green on CI-equivalent host)

@diegosouzapw
diegosouzapw merged commit b59127e into diegosouzapw:release/v3.8.49 Jul 27, 2026
15 checks passed
@backryun
backryun deleted the chore/ts7-types-media-result-unions branch July 27, 2026 14:37
@diegosouzapw diegosouzapw mentioned this pull request Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants