Skip to content

Pin imageDiscoverMemoryEstimate tests to Mac host#24

Merged
cryptopoly merged 1 commit intomainfrom
fix/test-host-platform-mock
May 1, 2026
Merged

Pin imageDiscoverMemoryEstimate tests to Mac host#24
cryptopoly merged 1 commit intomainfrom
fix/test-host-platform-mock

Conversation

@cryptopoly
Copy link
Copy Markdown
Owner

Summary

Three tests in `src/utils/tests/images.test.ts` have been failing on CI since 2026-04-29 (PR #20 video-gen merge). They pass locally on Mac developer boxes but fail on `ubuntu-latest` because:

  • `imageDiscoverMemoryEstimate(variant)` calls `assessImageGenerationSafety` with `device: null`.
  • That falls through to `inferDeviceFromHostPlatform()` (in `src/utils/videos.ts`) which inspects `navigator.userAgentData.platform` and returns `"mps"` on macOS, `"cuda"` everywhere else.
  • The expected values in the three failing tests were calibrated against the MPS path (FLUX text encoders, MPS runtime overhead, MPS-specific `runtimeFootprint` overrides).

CI failure summary on commit 92e6ca9:

```
modelFootprintGb 7.5 -> 4.5 (no MPS multiplier)
modelFootprintGb 41.3 -> 18.3 (no MPS GGUF overhead)
modelFootprintGb 20 -> 16 (skips runtimeFootprintMpsGb override)
```

Fix

Stub `navigator` + `navigator.userAgentData.platform` to `"macOS"` inside the `imageDiscoverMemoryEstimate` describe block via `beforeEach` + `vi.stubGlobal`. Restore globals after each test via `afterEach` + `vi.unstubAllGlobals`.

The `assessImageGenerationSafety` describe block above is untouched — it always passes an explicit `device`, so host-inference never fires there.

Test plan

  • `npm test` locally on macOS — 207/207 pass
  • CI on Linux — confirm 207/207 once this PR merges

Why now

Surfaced on the v0.7.2 smoke-test follow-up merges (#21, #22, #23). The bug pre-dates all three of those PRs but was on the list of CI failures inherited from PR #20.

The imageDiscoverMemoryEstimate describe block called the function with
no explicit device, which falls through to inferDeviceFromHostPlatform
in src/utils/videos.ts:

  - macOS host  -> 'mps' device path
  - Linux/Win   -> 'cuda' device path

The expected values in the three failing tests were calibrated against
the MPS path (FLUX text encoders, MPS runtime overhead, MPS-specific
runtimeFootprint overrides). On a Mac developer box they pass; on the
ubuntu-latest CI runner the host-inference returns 'cuda' and the
function picks the CUDA branch, producing different numbers:

  modelFootprintGb 7.5  -> 4.5   (loses MPS multiplier)
  modelFootprintGb 41.3 -> 18.3  (no MPS GGUF overhead)
  modelFootprintGb 20   -> 16    (skips runtimeFootprintMpsGb override)

This has been broken on CI since PR #20 (video-gen merge) introduced
the host-platform fallback. Every push to main since 2026-04-29 has
failed Build Desktop App > test on these three lines.

Fix
- Stub navigator + navigator.userAgentData.platform to 'macOS' inside
  the imageDiscoverMemoryEstimate describe block via beforeEach +
  vi.stubGlobal.
- Restore globals after each test via afterEach + vi.unstubAllGlobals.

The assessImageGenerationSafety describe block above is untouched
because it always passes an explicit device, so host-inference never
fires there.

Verified locally: all 207 tests pass on macOS.
@cryptopoly cryptopoly merged commit 3614be5 into main May 1, 2026
1 of 2 checks passed
cryptopoly added a commit that referenced this pull request May 1, 2026
PR #22's no-system-RAM-fallback path returns {'vram_total_gb': None}
on Linux CI runners (no torch.cuda, no nvidia-smi). The pre-existing
test_snapshot_vram_values_are_numeric required (int, float) which
breaks on those runners.

This fix originally landed in branch fix/test-host-platform-mock
(commit 3b147a9) but was pushed after PR #24 had already merged, so
only the imageDiscoverMemoryEstimate Mac pin (commit 7bbeeef) made
it into main. The orphan commit went unnoticed until run 25223969487
on this PR's first CI ride re-surfaced the same failure.

Loosen the type check to (int, float, type(None)) and rename the
test to ..._numeric_or_none so the intent is loud at the call site.
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.

1 participant