From 90df164a22089aa0c6cefdc7fc00ffdee54853a6 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 26 Aug 2026 22:03:15 +0000 Subject: [PATCH 1/2] fix(media-use): repoint the dead videogen tier, demote past unusable models `LOCAL_MODELS.videogen`'s `large` tier named `dgrauet/ltx-2.3-mlx-bf16`, which returns HTTP 401 and cannot be downloaded at all. It was not a dormant entry: `rankedByPreference` sorts by descending `needs.ramMB` when no `rank` is set, so the largest fitting tier is tried FIRST by design. Any machine clearing 32 GB *available* RAM selected the dead entry, `ltxVideoGenerate` caught the failure and returned a bare `null`, and since `ltx.local` is last in `["heygen.video", "ltx.local"]` and network providers are skipped under `--local-only` (`registry.mjs:206`), local video generation failed outright instead of falling back to the tier that works. It survived review because the table landed with "live verification on a 24GB M-series Mac" - and a 24 GB machine cannot select a 32 GB tier, so that entry was unreachable on the only machine that validated it. The unit fixtures inherit the same ceiling (`fittingSpecs` is 20000MB), so every existing test exercised the medium tier alone. Two changes: 1. Repoint to `dgrauet/ltx-2.3-mlx-q8` (reachable) and correct `sizeMB` from 45000 to 28800. Measured against the HF API: the q8 repo totals 87.5 GB, and the registry's own targeted `--include` subset is 28.76 GB. That matches the sibling q4 entry's convention (`sizeMB: 20000` vs a measured 19.48 GB subset), so 45000 was wrong under either reading. `--low-ram` is added because the entry's own note calls it required at this tier's 32 GB floor, and the invoke omitted it. 2. A repoint alone is one bad URL from a repeat, so add the missing recovery. `selectModelLadder` returns every fitting model best-first; `selectModel`'s pick is now defined as that list's head. All three sites that previously selected exactly one model and failed terminally walk the ladder instead, demoting past a tier that cannot run here - gated weights, runner off PATH, an OOM at a tier that nominally fits: - `ltx-video-provider.mjs` (videogen, the reported failure) - `mflux-provider.mjs` (imagegen - same shape, and its 32 GB/64 GB tiers are equally unverifiable on a 24 GB machine) - `local-run.mjs` (tts/asr/upscale - `fish-speech` missing should still get you Kokoro) Every demotion is logged rather than silent, so a quietly smaller model is never mistaken for the tier the machine nominally qualified for. Also fixes the `install` string both videogen entries share: it ended at `uv sync --all-extras`, which leaves the entry point in `.venv/bin`, so the "`ltx-2-mlx` not on PATH" hint named a command that following the instruction would not put on PATH. The q8 tier is NOT live-verified - no 32 GB+ Apple Silicon machine was available - and its notes say so. Shipping it unverified is safe precisely because of change 2: a wrong tier now costs one failed attempt, not the whole local path. - Rames Jusso Co-Authored-By: Claude Opus 5 (1M context) --- skills-manifest.json | 4 +- skills/media-use/scripts/lib/local-models.mjs | 47 ++++-- .../scripts/lib/local-models.test.mjs | 66 ++++++++ skills/media-use/scripts/lib/local-run.mjs | 66 ++++---- .../media-use/scripts/lib/local-run.test.mjs | 37 +++++ .../scripts/lib/ltx-video-provider.mjs | 106 +++++++------ .../scripts/lib/ltx-video-provider.test.mjs | 89 ++++++++++- .../media-use/scripts/lib/mflux-provider.mjs | 148 +++++++++++------- .../scripts/lib/mflux-provider.test.mjs | 115 ++++++++++++++ 9 files changed, 528 insertions(+), 150 deletions(-) create mode 100644 skills/media-use/scripts/lib/mflux-provider.test.mjs diff --git a/skills-manifest.json b/skills-manifest.json index 99ff50894f..fd44eaec7a 100644 --- a/skills-manifest.json +++ b/skills-manifest.json @@ -50,8 +50,8 @@ "files": 12 }, "media-use": { - "hash": "1b0ce647f5c7df95", - "files": 152 + "hash": "36eedcb77a5f40f3", + "files": 153 }, "motion-graphics": { "hash": "853ac75cbab69036", diff --git a/skills/media-use/scripts/lib/local-models.mjs b/skills/media-use/scripts/lib/local-models.mjs index af292ff324..ca392f7057 100644 --- a/skills/media-use/scripts/lib/local-models.mjs +++ b/skills/media-use/scripts/lib/local-models.mjs @@ -12,6 +12,10 @@ // (quality that is NOT size, e.g. ASR), else by RAM footprint (the quality // proxy for generation). No fit -> recommend the CLI/cloud path. // +// selectModelLadder() returns EVERY fitting model in that same order. Callers +// that can retry walk it so ONE unusable entry (gated weights, a missing +// binary, an OOM) demotes to the next tier instead of killing the local path. +// // Picks reflect the 2026 research pass, verified live where noted. export const CAPABILITIES = ["tts", "asr", "upscale", "videogen", "imagegen"]; @@ -102,13 +106,17 @@ const MODELS = { }, ], videogen: [ - // 2026-07 X research pass + live verification on a 24GB M-series Mac. + // 2026-07 X research pass + live verification on a 24GB M-series Mac - + // which reaches the q4 tier only: a 24GB machine cannot select the 32GB + // entry below it, so that tier's claims stay unverified until someone + // runs it on a 32GB+ machine. // The Mac-local video story is LTX 2.3 on MLX via dgrauet/ltx-2-mlx (the // pipeline these weights were converted for; also powers Phosphene). // Wan 2.x MLX exists only as A14B conversions (too large for consumer // unified memory); revisit when a 5B Wan MLX conversion lands. // IMPORTANT: download the weights with a targeted include list first; - // pointing tools at the repo blind snapshot-downloads all 60 GB: + // pointing tools at the repo blind snapshot-downloads the lot (60 GB q4, + // 88 GB q8): // hf download dgrauet/ltx-2.3-mlx-q4 --include \ // transformer-distilled-1.1.safetensors connector.safetensors \ // "vae_*.safetensors" audio_vae.safetensors vocoder.safetensors "*.json" @@ -119,24 +127,24 @@ const MODELS = { needs: { ramMB: 16384, gpu: true }, wordTimestamps: false, install: - "git clone https://github.com/dgrauet/ltx-2-mlx && cd ltx-2-mlx && uv sync --all-extras", + 'git clone https://github.com/dgrauet/ltx-2-mlx && cd ltx-2-mlx && uv sync --all-extras && export PATH="$PWD/.venv/bin:$PATH"', invoke: "ltx-2-mlx generate --prompt {prompt} --distilled --low-ram --model dgrauet/ltx-2.3-mlx-q4 --width {w} --height {h} --frames {frames} --frame-rate 24 --output {out}", notes: "LTX 2.3 int4 on MLX. Verified on 24GB unified: 512x320 x 33 frames in ~19 min cold (incl. text-encoder download), t2v with audio. Dims must be multiples of 64. i2v, retake/extend, keyframe interpolation supported.", }, { - id: "ltx-2.3-mlx-bf16", + id: "ltx-2.3-mlx-q8", tier: "large", - sizeMB: 45000, + sizeMB: 28800, needs: { ramMB: 32768, gpu: true }, wordTimestamps: false, install: - "git clone https://github.com/dgrauet/ltx-2-mlx && cd ltx-2-mlx && uv sync --all-extras", + 'git clone https://github.com/dgrauet/ltx-2-mlx && cd ltx-2-mlx && uv sync --all-extras && export PATH="$PWD/.venv/bin:$PATH"', invoke: - "ltx-2-mlx generate --prompt {prompt} --two-stage --model dgrauet/ltx-2.3-mlx-bf16 --width {w} --height {h} --frames {frames} --frame-rate 24 --output {out}", + "ltx-2-mlx generate --prompt {prompt} --two-stage --low-ram --model dgrauet/ltx-2.3-mlx-q8 --width {w} --height {h} --frames {frames} --frame-rate 24 --output {out}", notes: - "Full-precision two-stage pipeline (upstream production default). 32GB with --low-ram block streaming; 64-128GB Macs for long/HD runs (the 25s multi-scene spots seen in the wild).", + "LTX 2.3 int8 on MLX, two-stage (upstream production default; higher quality than the q4 distilled tier). Replaced dgrauet/ltx-2.3-mlx-bf16, which is gated (HTTP 401) and cannot be downloaded at all. sizeMB is the targeted include-list subset, measured 28.8GB. --low-ram matches this tier's 32GB floor (block streaming); 64-128GB Macs for long/HD runs. NOT live-verified on a 32GB+ machine - the q4 tier below is the verified one.", }, ], imagegen: [ @@ -247,6 +255,23 @@ function rankedByPreference(table) { }); } +/** + * Every local model for a capability this machine can actually run, best-first + * (same ordering as selectModel, whose pick is this list's head). + * + * Callers that can retry should walk the whole list: a table entry can be + * unusable for reasons no spec check can see - weights pulled or gated behind a + * login, the runner missing from PATH, an OOM at a tier that nominally fits. On + * a single-select call any one of those fails the entire local path, because the + * cascade cannot tell "this model is broken" from "nothing here fits you". + * Demoting to the next fitting tier is almost always what the user wanted. + */ +export function selectModelLadder(capability, specs, { preferTier } = {}) { + const table = tableFor(capability); + const pool = preferTier ? table.filter((m) => m.tier === preferTier) : table; + return rankedByPreference(pool).filter((model) => meetsSpecs(model, specs)); +} + /** * Pick the best local model the machine can run for a capability: the * highest-footprint model that fits the available-RAM budget (and GPU/VRAM). @@ -255,10 +280,8 @@ function rankedByPreference(table) { */ export function selectModel(capability, specs, { preferTier } = {}) { const table = tableFor(capability); - const pool = preferTier ? table.filter((m) => m.tier === preferTier) : table; - for (const model of rankedByPreference(pool)) { - if (meetsSpecs(model, specs)) return { model, tier: model.tier }; - } + const [model] = selectModelLadder(capability, specs, { preferTier }); + if (model) return { model, tier: model.tier }; const smallest = table.reduce((a, b) => (a.sizeMB <= b.sizeMB ? a : b)); return { recommend: "cli", diff --git a/skills/media-use/scripts/lib/local-models.test.mjs b/skills/media-use/scripts/lib/local-models.test.mjs index e1cc56368f..f1e8d0204f 100644 --- a/skills/media-use/scripts/lib/local-models.test.mjs +++ b/skills/media-use/scripts/lib/local-models.test.mjs @@ -4,6 +4,7 @@ import { listModels, meetsSpecs, selectModel, + selectModelLadder, describeModelLadder, CAPABILITIES, } from "./local-models.mjs"; @@ -152,3 +153,68 @@ test("ASR offers word-timestamp-capable models (better than plain whisper)", () "every ASR model must support word timestamps", ); }); + +// A machine that clears BOTH videogen tiers (the 32GB entry and the 16GB one). +// The existing fixtures deliberately sit under the large tier's floor, which is +// exactly how a dead 32GB entry stayed invisible: nothing could select it. +const bothVideogenTiers = { availableRamMB: 40000, gpu: { present: true } }; + +test("selectModelLadder returns every fitting model, best-first", () => { + const ladder = selectModelLadder("videogen", bothVideogenTiers); + assert.deepEqual( + ladder.map((m) => m.tier), + ["large", "medium"], + "both tiers fit 40GB, biggest first", + ); + assert.equal( + selectModel("videogen", bothVideogenTiers).model.id, + ladder[0].id, + "selectModel's pick is the ladder's head", + ); +}); + +test("selectModelLadder drops what the machine cannot run", () => { + const oneTier = selectModelLadder("videogen", { availableRamMB: 20000, gpu: { present: true } }); + assert.deepEqual( + oneTier.map((m) => m.tier), + ["medium"], + "20GB cannot reach the 32GB tier", + ); + assert.deepEqual( + selectModelLadder("videogen", { availableRamMB: 100, gpu: { present: true } }), + [], + "nothing fits -> empty ladder, and selectModel recommends the CLI", + ); + assert.equal( + selectModel("videogen", { availableRamMB: 100, gpu: { present: true } }).recommend, + "cli", + ); +}); + +test("selectModelLadder honours preferTier", () => { + const pinned = selectModelLadder("videogen", bothVideogenTiers, { preferTier: "medium" }); + assert.deepEqual( + pinned.map((m) => m.tier), + ["medium"], + "preferTier pins the ladder to one tier", + ); +}); + +test("an invoke that names an owner/repo model agrees with the entry id", () => { + // Guards a half-done repoint: moving an entry to different weights means + // changing BOTH the id and the --model argument. Change one and the table + // selects one model while the runner downloads another. + let checked = 0; + for (const cap of CAPABILITIES) { + for (const m of listModels(cap)) { + if (m.repo) continue; // entries with an explicit repo resolve through it + const named = /--model\s+(\S+)/.exec(m.invoke); + if (!named) continue; + const [, name] = named[1].split("/"); + if (!name) continue; // a bare model name, not an owner/repo id + assert.equal(name, m.id, `${cap}/${m.id}: invoke runs ${named[1]}`); + checked += 1; + } + } + assert.ok(checked > 0, "no entry pins an owner/repo model - guard would be vacuous"); +}); diff --git a/skills/media-use/scripts/lib/local-run.mjs b/skills/media-use/scripts/lib/local-run.mjs index b8b1bc6c21..4550f10f97 100644 --- a/skills/media-use/scripts/lib/local-run.mjs +++ b/skills/media-use/scripts/lib/local-run.mjs @@ -1,10 +1,12 @@ import { execFileSync } from "node:child_process"; -import { selectModel } from "./local-models.mjs"; +import { selectModel, selectModelLadder } from "./local-models.mjs"; import { probeSpecs } from "./specs.mjs"; // Run a USER-INSTALLED local model for a capability (tts/asr/upscale). -// Picks the best tier the machine supports (selectModel), checks the tool is on -// PATH, fills the model's invoke template, and runs it. Returns: +// Walks the tiers the machine supports best-first (selectModelLadder), checking +// the tool is on PATH, filling the model's invoke template, and running it. A +// tier whose tool is missing or whose run fails demotes to the next fitting +// tier, so one unusable entry does not fail the capability. Returns: // { model, tier, out } on success // { recommend:"install", model, command, reason } when the tool isn't installed // { recommend:"cli", reason } when no tier fits the machine @@ -35,30 +37,40 @@ export function runLocalModel(capability, opts = {}) { vars = {}, preferTier, } = opts; - const sel = selectModel(capability, specs, { preferTier }); - if (sel.recommend) return sel; // no tier fits -> recommend the CLI path + const ladder = selectModelLadder(capability, specs, { preferTier }); + // no tier fits at all -> recommend the CLI path (selectModel words the reason) + if (!ladder.length) return selectModel(capability, specs, { preferTier }); - const { model } = sel; - const bin = model.invoke.split(/\s+/)[0]; - try { - which(bin); - } catch { - return { - recommend: "install", - model: model.id, - command: model.install, - reason: `${model.id} not installed`, - }; + // Best tier first, demoting past any tier that cannot run here: a missing + // tool or a failed run at the top tier must not hide a lower tier that works + // (fish-speech absent should still get you Kokoro). The last tier's failure is + // what gets reported, since by then nothing local ran. + let lastFailure = null; + for (const model of ladder) { + const bin = model.invoke.split(/\s+/)[0]; + try { + which(bin); + } catch { + lastFailure = { + recommend: "install", + model: model.id, + command: model.install, + reason: `${model.id} not installed`, + }; + continue; + } + try { + exec(fill(model.invoke, vars)); + } catch (e) { + lastFailure = { + recommend: "install", + model: model.id, + command: model.install, + reason: e.message || String(e), + }; + continue; + } + return { model: model.id, tier: model.tier, out: vars.out }; } - try { - exec(fill(model.invoke, vars)); - } catch (e) { - return { - recommend: "install", - model: model.id, - command: model.install, - reason: e.message || String(e), - }; - } - return { model: model.id, tier: sel.tier, out: vars.out }; + return lastFailure; } diff --git a/skills/media-use/scripts/lib/local-run.test.mjs b/skills/media-use/scripts/lib/local-run.test.mjs index 7e9c883dbc..b88b24188a 100644 --- a/skills/media-use/scripts/lib/local-run.test.mjs +++ b/skills/media-use/scripts/lib/local-run.test.mjs @@ -52,3 +52,40 @@ test("a failing run degrades to an install recommendation, never throws", () => }); assert.equal(r.recommend, "install"); }); + +test("a tier whose tool is missing demotes to the next tier that fits", () => { + // 64GB + GPU fits BOTH tts tiers, so the ladder has two rungs: fish-speech + // (its own binary) above Kokoro (`python -m kokoro`). fish-speech absent must + // not cost the user Kokoro. + const strongGpu = { ramMB: 64000, gpu: { present: true, vramMB: 24000 } }; + let ran = ""; + const r = runLocalModel("tts", { + specs: strongGpu, + which: (bin) => { + if (bin === "fish-speech") throw new Error("not found"); + }, + exec: (cmd) => { + ran = cmd; + }, + vars: { text: "hello", voice: "af_heart", out: "/tmp/v.wav" }, + }); + + assert.equal(r.model, "kokoro", "demoted past the missing fish-speech binary"); + assert.equal(r.tier, "medium"); + assert.match(ran, /kokoro/); +}); + +test("every fitting tier failing reports the last tier's install command", () => { + const strongGpu = { ramMB: 64000, gpu: { present: true, vramMB: 24000 } }; + const r = runLocalModel("tts", { + specs: strongGpu, + which: ok, + exec: () => { + throw new Error("boom"); + }, + vars: { text: "hi", out: "/tmp/v.wav" }, + }); + + assert.equal(r.recommend, "install"); + assert.equal(r.model, "kokoro", "the smallest fitting tier is the actionable one"); +}); diff --git a/skills/media-use/scripts/lib/ltx-video-provider.mjs b/skills/media-use/scripts/lib/ltx-video-provider.mjs index e7ca747489..9fb9a6755a 100644 --- a/skills/media-use/scripts/lib/ltx-video-provider.mjs +++ b/skills/media-use/scripts/lib/ltx-video-provider.mjs @@ -3,7 +3,7 @@ import { existsSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { probeSpecs } from "./specs.mjs"; -import { buildArgv, selectModel } from "./local-models.mjs"; +import { buildArgv, selectModel, selectModelLadder } from "./local-models.mjs"; export async function ltxVideoGenerate( intent, @@ -12,59 +12,69 @@ export async function ltxVideoGenerate( pathExists = existsSync, ) { const specs = ctx?.specs || probeSpecs(); - const sel = selectModel("videogen", specs, { preferTier: ctx?.preferTier }); - if (sel.recommend) { + const ladder = selectModelLadder("videogen", specs, { preferTier: ctx?.preferTier }); + if (!ladder.length) { + const { reason } = selectModel("videogen", specs, { preferTier: ctx?.preferTier }); console.error( - `media-use: local video gen not enabled (${sel.reason}). Enable a fitting free on-device LTX model to use this provider.`, + `media-use: local video gen not enabled (${reason}). Enable a fitting free on-device LTX model to use this provider.`, ); return null; } - const { model } = sel; - const bin = model.invoke.trim().split(/\s+/)[0]; - try { - execFn("which", [bin], { stdio: ["ignore", "ignore", "ignore"] }); - } catch { - console.error( - `media-use: local video gen not enabled (\`${bin}\` not on PATH). Install for free on-device LTX: ${model.install}`, - ); - return null; - } - - const outPath = join(tmpdir(), `media-use-ltx-${process.pid}-${Date.now()}.mp4`); - const width = ctx?.width || 512; - const height = ctx?.height || 320; - const frames = ctx?.frames || 33; - const argv = buildArgv(model.invoke, { - prompt: intent, - w: width, - h: height, - frames, - out: outPath, - }); - argv.shift(); + // Walk the whole ladder, best tier first. A tier that cannot run on this + // machine for a reason no spec check sees (runner off PATH, gated weights, an + // OOM) demotes to the next fitting tier instead of failing local video gen + // outright. Every demotion is reported: a silent drop to a smaller model + // leaves the caller wondering why the output looks the way it does. + for (const model of ladder) { + const bin = model.invoke.trim().split(/\s+/)[0]; + try { + execFn("which", [bin], { stdio: ["ignore", "ignore", "ignore"] }); + } catch { + console.error( + `media-use: local video gen not enabled (\`${bin}\` not on PATH). Install for free on-device LTX: ${model.install}`, + ); + continue; + } - try { - execFn(bin, argv, { - encoding: "utf8", - timeout: 1_800_000, - stdio: ["ignore", "pipe", "pipe"], + const outPath = join(tmpdir(), `media-use-ltx-${process.pid}-${Date.now()}.mp4`); + const argv = buildArgv(model.invoke, { + prompt: intent, + w: ctx?.width || 512, + h: ctx?.height || 320, + frames: ctx?.frames || 33, + out: outPath, }); - } catch (err) { - console.error( - `media-use: local video gen (${model.id}) failed: ${err.stderr?.toString().trim().slice(-200) || err.message}`, - ); - return null; + argv.shift(); + + try { + execFn(bin, argv, { + encoding: "utf8", + timeout: 1_800_000, + stdio: ["ignore", "pipe", "pipe"], + }); + } catch (err) { + console.error( + `media-use: local video gen (${model.id}) failed: ${err.stderr?.toString().trim().slice(-200) || err.message}`, + ); + continue; + } + if (!pathExists(outPath)) { + console.error( + `media-use: local video gen (${model.id}) exited cleanly but wrote no output file`, + ); + continue; + } + return { + localPath: outPath, + ext: ".mp4", + source: "generated", + metadata: { + description: intent, + provider: "ltx.local", + provenance: { prompt: intent }, + }, + }; } - if (!pathExists(outPath)) return null; - return { - localPath: outPath, - ext: ".mp4", - source: "generated", - metadata: { - description: intent, - provider: "ltx.local", - provenance: { prompt: intent }, - }, - }; + return null; } diff --git a/skills/media-use/scripts/lib/ltx-video-provider.test.mjs b/skills/media-use/scripts/lib/ltx-video-provider.test.mjs index b24e35cda5..bbd7a0112c 100644 --- a/skills/media-use/scripts/lib/ltx-video-provider.test.mjs +++ b/skills/media-use/scripts/lib/ltx-video-provider.test.mjs @@ -124,7 +124,10 @@ test("generate failure returns null instead of throwing", async (t) => { assert.equal(calls, 2); }); -test("missing generated output returns null", async () => { +test("missing generated output returns null and says so", async (t) => { + const errors = []; + t.mock.method(console, "error", (message) => errors.push(message)); + const result = await ltxVideoGenerate( "storm clouds", { specs: fittingSpecs }, @@ -133,4 +136,88 @@ test("missing generated output returns null", async () => { ); assert.equal(result, null); + assert.equal(errors.length, 1); + assert.match(errors[0], /wrote no output file/); +}); + +// 40GB clears BOTH videogen tiers, so the ladder has two rungs. `fittingSpecs` +// above sits under the large tier's floor on purpose: every other test in this +// file exercises the medium tier alone, which is precisely why a broken large +// tier could sit in the table unnoticed. +const bothTiersSpecs = { availableRamMB: 40000, gpu: { present: true } }; + +const isGenerate = (call) => call[0] !== "which"; + +test("a top tier that cannot run demotes to the next fitting tier", async (t) => { + const errors = []; + t.mock.method(console, "error", (message) => errors.push(message)); + const calls = []; + // The runner is installed, but the large tier's weights are gated: the + // download 401s and `generate` exits non-zero. The medium tier then works. + const fakeExec = (...call) => { + calls.push(call); + if (isGenerate(call) && call[1].includes("dgrauet/ltx-2.3-mlx-q8")) { + const err = new Error("exit 1"); + err.stderr = "401 Client Error: Unauthorized for url: .../ltx-2.3-mlx-q8"; + throw err; + } + }; + + const result = await ltxVideoGenerate( + "storm clouds", + { specs: bothTiersSpecs }, + fakeExec, + () => true, + ); + + assert.ok(result, "the medium tier still produced a video"); + const generated = calls.filter(isGenerate).map((call) => call[1].join(" ")); + assert.equal(generated.length, 2, "large attempted first, then medium"); + assert.match(generated[0], /dgrauet\/ltx-2\.3-mlx-q8/); + assert.match(generated[1], /dgrauet\/ltx-2\.3-mlx-q4/); + // the demotion is reported, never silent: a smaller model changes the output + assert.equal(errors.length, 1); + assert.match(errors[0], /ltx-2\.3-mlx-q8\) failed/); + assert.match(errors[0], /401/); +}); + +test("every fitting tier failing returns null, one reason per tier", async (t) => { + const errors = []; + t.mock.method(console, "error", (message) => errors.push(message)); + const fakeExec = (...call) => { + if (isGenerate(call)) throw new Error("mlx out of memory"); + }; + + const result = await ltxVideoGenerate( + "storm clouds", + { specs: bothTiersSpecs }, + fakeExec, + () => true, + ); + + assert.equal(result, null); + assert.equal(errors.length, 2, "both tiers tried, both reported"); + assert.match(errors[0], /ltx-2\.3-mlx-q8/); + assert.match(errors[1], /ltx-2\.3-mlx-q4/); +}); + +test("preferTier pins the attempt to one tier instead of demoting", async (t) => { + t.mock.method(console, "error", () => {}); + const calls = []; + const fakeExec = (...call) => { + calls.push(call); + if (isGenerate(call)) throw new Error("boom"); + }; + + const result = await ltxVideoGenerate( + "storm clouds", + { specs: bothTiersSpecs, preferTier: "large" }, + fakeExec, + () => true, + ); + + assert.equal(result, null); + const generated = calls.filter(isGenerate).map((call) => call[1].join(" ")); + assert.equal(generated.length, 1, "pinned to large: no demotion to medium"); + assert.match(generated[0], /dgrauet\/ltx-2\.3-mlx-q8/); }); diff --git a/skills/media-use/scripts/lib/mflux-provider.mjs b/skills/media-use/scripts/lib/mflux-provider.mjs index 9de2f920bf..cc7157a07f 100644 --- a/skills/media-use/scripts/lib/mflux-provider.mjs +++ b/skills/media-use/scripts/lib/mflux-provider.mjs @@ -3,13 +3,14 @@ import { existsSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { probeSpecs } from "./specs.mjs"; -import { buildArgv, selectModel } from "./local-models.mjs"; +import { buildArgv, selectModelLadder } from "./local-models.mjs"; // Local image generation via mflux (FLUX-on-MLX), the Mac-native runner. -// Spec-gated: selectModel("imagegen", specs) returns the best FLUX-class model -// the machine's AVAILABLE RAM can actually run (medium FLUX-schnell --low-ram on -// ~24GB, up to Qwen-Image on 64GB+). When nothing local fits, this returns null -// so the registry falls through to the codex image upsell. +// Spec-gated: selectModelLadder("imagegen", specs) returns every FLUX-class +// model the machine's AVAILABLE RAM can actually run (medium FLUX-schnell +// --low-ram on ~24GB, up to Qwen-Image on 64GB+), best first. When nothing +// local fits, or no fitting tier can actually run here, this returns null so +// the registry falls through to the codex image upsell. // // The official FLUX repos are HF-gated, so the model entries point --path at // non-gated community 4-bit re-uploads; the repo is resolved to a local snapshot @@ -17,70 +18,97 @@ import { buildArgv, selectModel } from "./local-models.mjs"; // Resolve an HF repo to its local snapshot dir. `hf download` is idempotent and // prints the snapshot path as its last line. -function resolveSnapshot(repo) { - const out = execFileSync("hf", ["download", repo], { +function resolveSnapshot(repo, execFn, pathExists) { + const out = execFn("hf", ["download", repo], { encoding: "utf8", timeout: 1_800_000, stdio: ["ignore", "pipe", "pipe"], }); - const path = out.trim().split(/\r?\n/).pop()?.trim(); - return path && existsSync(path) ? path : null; + const path = out?.trim().split(/\r?\n/).pop()?.trim(); + return path && pathExists(path) ? path : null; } -export async function mfluxImageGenerate(intent, ctx) { +export async function mfluxImageGenerate( + intent, + ctx, + execFn = execFileSync, + pathExists = existsSync, +) { const specs = ctx?.specs || probeSpecs(); - const sel = selectModel("imagegen", specs, { preferTier: ctx?.preferTier }); - if (sel.recommend) return null; // no local model fits -> codex upsell/fallback + const ladder = selectModelLadder("imagegen", specs, { preferTier: ctx?.preferTier }); + if (!ladder.length) return null; // no local model fits -> codex upsell/fallback - const { model } = sel; - const bin = model.invoke.trim().split(/\s+/)[0]; - // Not installed? Surface the exact enable-command FIRST (before the model - // download) so the agent learns the free local path is available instead of - // silently taking the codex upsell. - try { - execFileSync("which", [bin], { stdio: ["ignore", "ignore", "ignore"] }); - } catch { - console.error( - `media-use: local image gen not enabled (\`${bin}\` not on PATH). Install for free on-device FLUX: ${model.install}`, - ); - return null; - } - - const outPath = join(tmpdir(), `media-use-mflux-${process.pid}-${Date.now()}.png`); - const width = ctx?.width || 512; - const height = ctx?.height || 512; - const seed = ctx?.seed ?? 42; + // Best tier first, demoting past any tier that cannot run here (runner off + // PATH, a snapshot that won't download, an OOM) rather than failing local + // image gen outright. Every demotion is reported, so a quietly smaller model + // is never mistaken for the tier the machine nominally qualified for. + for (const model of ladder) { + const bin = model.invoke.trim().split(/\s+/)[0]; + // Not installed? Surface the exact enable-command (before the model + // download) so the agent learns the free local path is available instead of + // silently taking the codex upsell. + try { + execFn("which", [bin], { stdio: ["ignore", "ignore", "ignore"] }); + } catch { + console.error( + `media-use: local image gen not enabled (\`${bin}\` not on PATH). Install for free on-device FLUX: ${model.install}`, + ); + continue; + } - const vars = { prompt: intent, w: width, h: height, seed, out: outPath }; - if (model.repo && model.invoke.includes("{model_path}")) { - const snap = model.repo ? resolveSnapshot(model.repo) : null; - if (!snap) return null; - vars.model_path = snap; - } + const outPath = join(tmpdir(), `media-use-mflux-${process.pid}-${Date.now()}.png`); + const vars = { + prompt: intent, + w: ctx?.width || 512, + h: ctx?.height || 512, + seed: ctx?.seed ?? 42, + out: outPath, + }; + if (model.repo && model.invoke.includes("{model_path}")) { + let snap = null; + let why = `could not resolve a local snapshot of ${model.repo}`; + try { + snap = resolveSnapshot(model.repo, execFn, pathExists); + } catch (err) { + why = `hf download failed: ${err.stderr?.toString().trim().slice(-200) || err.message}`; + } + if (!snap) { + console.error(`media-use: local image gen (${model.id}): ${why}`); + continue; + } + vars.model_path = snap; + } - const argv = buildArgv(model.invoke, vars); - argv.shift(); // drop the bin (already validated) - try { - execFileSync(bin, argv, { - encoding: "utf8", - timeout: 1_800_000, - stdio: ["ignore", "pipe", "pipe"], - }); - } catch (err) { - console.error( - `media-use: local image gen (${model.id}) failed: ${err.stderr?.toString().trim().slice(-200) || err.message}`, - ); - return null; + const argv = buildArgv(model.invoke, vars); + argv.shift(); // drop the bin (already validated) + try { + execFn(bin, argv, { + encoding: "utf8", + timeout: 1_800_000, + stdio: ["ignore", "pipe", "pipe"], + }); + } catch (err) { + console.error( + `media-use: local image gen (${model.id}) failed: ${err.stderr?.toString().trim().slice(-200) || err.message}`, + ); + continue; + } + if (!pathExists(outPath)) { + console.error( + `media-use: local image gen (${model.id}) exited cleanly but wrote no output file`, + ); + continue; + } + return { + localPath: outPath, + ext: ".png", + source: "generated", + metadata: { + description: intent, + provider: `mflux.${model.id}`, + provenance: { model: model.id, tier: model.tier, prompt: intent }, + }, + }; } - if (!existsSync(outPath)) return null; - return { - localPath: outPath, - ext: ".png", - source: "generated", - metadata: { - description: intent, - provider: `mflux.${model.id}`, - provenance: { model: model.id, tier: model.tier, prompt: intent }, - }, - }; + return null; } diff --git a/skills/media-use/scripts/lib/mflux-provider.test.mjs b/skills/media-use/scripts/lib/mflux-provider.test.mjs new file mode 100644 index 0000000000..9bb89c00a5 --- /dev/null +++ b/skills/media-use/scripts/lib/mflux-provider.test.mjs @@ -0,0 +1,115 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mfluxImageGenerate } from "./mflux-provider.mjs"; + +// 40GB clears the 32GB klein tier and the 8GB schnell tier below it; the 64GB +// qwen tier stays out of reach. Two rungs is what makes demotion observable. +const bothTiersSpecs = { availableRamMB: 40000, gpu: { present: true } }; +const SNAPSHOT = "/tmp/hf-snapshot"; + +// exec stub covering all three shells-out mflux does: the PATH probe, the +// idempotent `hf download`, and the generate itself. +function stubExec({ failGenerateFor = [], failWhichFor = [] } = {}) { + const calls = []; + const exec = (...call) => { + calls.push(call); + const [bin, argv] = call; + if (bin === "which") { + if (failWhichFor.includes(argv[0])) throw new Error("not found"); + return ""; + } + if (bin === "hf") return `Fetching 6 files...\n${SNAPSHOT}\n`; + if (failGenerateFor.some((id) => argv.join(" ").includes(id))) { + const err = new Error("exit 1"); + err.stderr = "mlx.core.metal: out of memory"; + throw err; + } + return ""; + }; + return { calls, exec }; +} + +const generateCalls = (calls) => + calls.filter(([bin]) => bin !== "which" && bin !== "hf").map(([, argv]) => argv.join(" ")); + +test("no local model fits: falls through to the upsell without shelling out", async () => { + const { calls, exec } = stubExec(); + + const result = await mfluxImageGenerate( + "a red bicycle", + { specs: { availableRamMB: 100, gpu: { present: true } } }, + exec, + () => true, + ); + + assert.equal(result, null); + assert.deepEqual(calls, []); +}); + +test("a top tier that cannot run demotes to the next fitting tier", async (t) => { + const errors = []; + t.mock.method(console, "error", (message) => errors.push(message)); + const { calls, exec } = stubExec({ failGenerateFor: ["flux2-klein-4b"] }); + + const result = await mfluxImageGenerate( + "a red bicycle", + { specs: bothTiersSpecs }, + exec, + () => true, + ); + + assert.ok(result, "the schnell tier still produced an image"); + assert.equal(result.metadata.provider, "mflux.flux-schnell-mflux-q4"); + const generated = generateCalls(calls); + assert.equal(generated.length, 2, "klein attempted first, then schnell"); + assert.match(generated[0], /flux2-klein-4b/); + assert.match(generated[1], /--model schnell/); + assert.equal(errors.length, 1, "the demotion is reported, not silent"); + assert.match(errors[0], /flux2-klein-mflux-q4\) failed/); +}); + +test("a snapshot that will not resolve demotes rather than failing outright", async (t) => { + const errors = []; + t.mock.method(console, "error", (message) => errors.push(message)); + const calls = []; + const exec = (...call) => { + calls.push(call); + const [bin, argv] = call; + if (bin === "which") return ""; + // klein's weights won't download; schnell's do + if (bin === "hf") { + if (argv[1].includes("FLUX.2-klein")) throw new Error("403 Forbidden"); + return `${SNAPSHOT}\n`; + } + return ""; + }; + + const result = await mfluxImageGenerate( + "a red bicycle", + { specs: bothTiersSpecs }, + exec, + () => true, + ); + + assert.ok(result, "demoted past the ungettable weights"); + assert.equal(result.metadata.provider, "mflux.flux-schnell-mflux-q4"); + assert.equal(errors.length, 1); + assert.match(errors[0], /hf download failed/); +}); + +test("runner missing from PATH reports the install hint per tier and returns null", async (t) => { + const errors = []; + t.mock.method(console, "error", (message) => errors.push(message)); + const { exec } = stubExec({ failWhichFor: ["mflux-generate"] }); + + const result = await mfluxImageGenerate( + "a red bicycle", + { specs: bothTiersSpecs }, + exec, + () => true, + ); + + assert.equal(result, null); + assert.equal(errors.length, 2, "both fitting tiers reported"); + assert.match(errors[0], /uv pip install mflux/); +}); From 6195d5a7f2caf65aa8eadbe864d66ddf4b436a22 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 26 Aug 2026 22:42:54 +0000 Subject: [PATCH 2/2] fix(media-use): report the real videogen download size, disclose it, discard failed partials Addresses review feedback on #3509 (CHANGES_REQUESTED at 90df164a), plus the follow-on ask to tell the user what a download costs before they accept it. 1. `sizeMB` described a targeted `--include` subset that no run ever gets. Both videogen invokes pass a repo id to `--model`, and upstream `resolve_model_dir()` (`ltx_pipelines_mlx/utils/_orchestration.py:35-40`) calls `snapshot_download(repo)` with no `allow_patterns`, so the full repo lands regardless of what was pre-fetched. Corrected to measured repo totals: q8 87500 (87,511,991,375 B) and q4 59700 (59,686,429,583 B). q4 was wrong the same way at 20000, so both are fixed together rather than leaving one convention on each side. My earlier claim that 28800 "matches the sibling q4 entry's convention" was wrong in the way that matters: the convention itself described a subset the runner does not honor. The file's own comment already said "blind snapshot-downloads the lot (60 GB q4, 88 GB q8)" three lines above the fields that contradicted it, and the original report measured it too ("the q4 cache ended at 56 GB and q8 at 82 GB"), which reconciles exactly once read as GiB: 59.69 GB = 55.6 GiB, 87.51 GB = 81.5 GiB. So the download is the complete repo both times, not a partial fetch. Removed the `--include` recipe rather than repairing it: it is ineffective (the runner refetches at generate time) and insufficient (`--two-stage` is "dev model + CFG at half-res, upscale, distilled LoRA refine" per upstream's own help text, so it needs transformer-dev AND transformer-distilled AND spatial_upscaler_x2; `--distilled` needs an upscaler too). The q4 tier verified on a 24 GB Mac only worked BECAUSE the download is unfiltered. 2. Nothing told the user what they were agreeing to before a tool started pulling tens of GB. `describeDownload()` in `specs.mjs` names the size and the directory the weights land in, and checks free space with `statfs` against that directory rather than cwd, since the weights do not land in cwd. A tier that will not fit is still offered, with a plain statement that it will not fit: hiding it would make a machine that could free up space look like it has no large tier. Unknown free space reports as unknown, not as zero. Wired into both providers' install hints, the `runLocalModel` install payload (now carrying `sizeMB`), and `describeModelLadder`. 3. Each retry attempt mints its own timestamped temp path, so a partial artifact from a failed tier was orphaned rather than overwritten, and a lower tier then succeeding hid it. Both providers discard the partial before demoting, guarded so a file that cannot be removed never masks the generate failure it came from. Video is the material case: a partial mp4 is large. `local-run.mjs` is deliberately unchanged here. Its `out` is caller-provided and identical across attempts, so a partial is overwritten rather than orphaned, and unlinking a path the caller named would be a footgun. The rule the two providers follow is: clean up what you allocate. Tests: 553/553 across `skills/**/*.test.mjs` (+15). New coverage pins the cleanup (failed tier's partial removed, returned artifact survives, one discard per attempt on the all-fail path, an unremovable partial still surfaces the real failure) and the disclosure (cache-dir precedence, statfs walk-up to the deepest existing ancestor, unknown-vs-zero, and the will-not-fit wording). Every new guard mutation-tested: removing any one of them turns tests red. - Rames Jusso Co-Authored-By: Claude Opus 5 (1M context) --- skills-manifest.json | 2 +- skills/media-use/scripts/lib/local-models.mjs | 23 +++-- skills/media-use/scripts/lib/local-run.mjs | 6 +- .../media-use/scripts/lib/local-run.test.mjs | 30 ++++++ .../scripts/lib/ltx-video-provider.mjs | 20 +++- .../scripts/lib/ltx-video-provider.test.mjs | 93 +++++++++++++++++++ .../media-use/scripts/lib/mflux-provider.mjs | 20 +++- .../scripts/lib/mflux-provider.test.mjs | 59 ++++++++++++ skills/media-use/scripts/lib/specs.mjs | 51 ++++++++++ skills/media-use/scripts/lib/specs.test.mjs | 84 ++++++++++++++++- 10 files changed, 369 insertions(+), 19 deletions(-) diff --git a/skills-manifest.json b/skills-manifest.json index fd44eaec7a..149190844a 100644 --- a/skills-manifest.json +++ b/skills-manifest.json @@ -50,7 +50,7 @@ "files": 12 }, "media-use": { - "hash": "36eedcb77a5f40f3", + "hash": "00a2d26e22fc1ab1", "files": 153 }, "motion-graphics": { diff --git a/skills/media-use/scripts/lib/local-models.mjs b/skills/media-use/scripts/lib/local-models.mjs index ca392f7057..28b8715beb 100644 --- a/skills/media-use/scripts/lib/local-models.mjs +++ b/skills/media-use/scripts/lib/local-models.mjs @@ -114,16 +114,20 @@ const MODELS = { // pipeline these weights were converted for; also powers Phosphene). // Wan 2.x MLX exists only as A14B conversions (too large for consumer // unified memory); revisit when a 5B Wan MLX conversion lands. - // IMPORTANT: download the weights with a targeted include list first; - // pointing tools at the repo blind snapshot-downloads the lot (60 GB q4, - // 88 GB q8): - // hf download dgrauet/ltx-2.3-mlx-q4 --include \ - // transformer-distilled-1.1.safetensors connector.safetensors \ - // "vae_*.safetensors" audio_vae.safetensors vocoder.safetensors "*.json" + // IMPORTANT: sizeMB below is the FULL repo, because that is what a run + // actually downloads. Both invokes pass a repo id to `--model`, and + // upstream resolve_model_dir() (ltx_pipelines_mlx/utils/_orchestration.py) + // calls snapshot_download(repo) with no allow_patterns - so the whole repo + // lands regardless of what you pre-fetched. A targeted `hf download + // --include` subset used to be documented here; it was removed because it + // is both ineffective (the runner refetches the rest at generate time) and + // insufficient (--two-stage needs transformer-dev AND transformer-distilled + // AND the x2 spatial upscaler; --distilled needs an upscaler too). The q4 + // tier verified below only worked BECAUSE the download is unfiltered. { id: "ltx-2.3-mlx-q4", tier: "medium", - sizeMB: 20000, // distilled subset; gemma-3-12b-4bit text encoder adds ~7GB + sizeMB: 59700, // full repo, measured 59.69GB; gemma-3-12b-4bit text encoder adds ~7GB needs: { ramMB: 16384, gpu: true }, wordTimestamps: false, install: @@ -136,7 +140,7 @@ const MODELS = { { id: "ltx-2.3-mlx-q8", tier: "large", - sizeMB: 28800, + sizeMB: 87500, // full repo, measured 87.51GB needs: { ramMB: 32768, gpu: true }, wordTimestamps: false, install: @@ -144,7 +148,7 @@ const MODELS = { invoke: "ltx-2-mlx generate --prompt {prompt} --two-stage --low-ram --model dgrauet/ltx-2.3-mlx-q8 --width {w} --height {h} --frames {frames} --frame-rate 24 --output {out}", notes: - "LTX 2.3 int8 on MLX, two-stage (upstream production default; higher quality than the q4 distilled tier). Replaced dgrauet/ltx-2.3-mlx-bf16, which is gated (HTTP 401) and cannot be downloaded at all. sizeMB is the targeted include-list subset, measured 28.8GB. --low-ram matches this tier's 32GB floor (block streaming); 64-128GB Macs for long/HD runs. NOT live-verified on a 32GB+ machine - the q4 tier below is the verified one.", + "LTX 2.3 int8 on MLX, two-stage (upstream production default; higher quality than the q4 distilled tier). Replaced dgrauet/ltx-2.3-mlx-bf16, which is gated (HTTP 401) and cannot be downloaded at all. Costs an 87.5GB download against q4's 59.7GB - a real tradeoff, not a rounding difference. --two-stage is dev model + CFG at half-res, upscale, then distilled LoRA refine (upstream's own help text), so it needs transformer-dev + transformer-distilled + spatial_upscaler_x2; the full snapshot carries all three. --low-ram matches this tier's 32GB floor (block streaming); 64-128GB Macs for long/HD runs. NOT live-verified on a 32GB+ machine - the q4 tier below is the verified one.", }, ], imagegen: [ @@ -303,6 +307,7 @@ export function describeModelLadder(capability, specs) { id: model.id, tier: model.tier, needsRamMB: model.needs?.ramMB ?? 0, + sizeMB: model.sizeMB, fits, reason: fits ? `fits (needs ~${model.needs?.ramMB}MB, ${budget}MB available)` diff --git a/skills/media-use/scripts/lib/local-run.mjs b/skills/media-use/scripts/lib/local-run.mjs index 4550f10f97..1692917f2c 100644 --- a/skills/media-use/scripts/lib/local-run.mjs +++ b/skills/media-use/scripts/lib/local-run.mjs @@ -8,7 +8,7 @@ import { probeSpecs } from "./specs.mjs"; // tier whose tool is missing or whose run fails demotes to the next fitting // tier, so one unusable entry does not fail the capability. Returns: // { model, tier, out } on success -// { recommend:"install", model, command, reason } when the tool isn't installed +// { recommend:"install", model, sizeMB, command, reason } tool isn't installed // { recommend:"cli", reason } when no tier fits the machine // `exec` / `which` are injectable for tests. // @@ -54,8 +54,9 @@ export function runLocalModel(capability, opts = {}) { lastFailure = { recommend: "install", model: model.id, + sizeMB: model.sizeMB, command: model.install, - reason: `${model.id} not installed`, + reason: `${model.id} not installed (~${(model.sizeMB / 1000).toFixed(1)}GB to download once it is)`, }; continue; } @@ -65,6 +66,7 @@ export function runLocalModel(capability, opts = {}) { lastFailure = { recommend: "install", model: model.id, + sizeMB: model.sizeMB, command: model.install, reason: e.message || String(e), }; diff --git a/skills/media-use/scripts/lib/local-run.test.mjs b/skills/media-use/scripts/lib/local-run.test.mjs index b88b24188a..d9dbab29e8 100644 --- a/skills/media-use/scripts/lib/local-run.test.mjs +++ b/skills/media-use/scripts/lib/local-run.test.mjs @@ -89,3 +89,33 @@ test("every fitting tier failing reports the last tier's install command", () => assert.equal(r.recommend, "install"); assert.equal(r.model, "kokoro", "the smallest fitting tier is the actionable one"); }); + +test("the install recommendation states the download size", () => { + // nothing today tells the user what they are agreeing to before a tool + // starts pulling weights, so the size travels in the payload AND in the + // text a caller shows them + const r = runLocalModel("tts", { + specs: strongCpu, + which: () => { + throw new Error("not found"); + }, + }); + + assert.equal(r.recommend, "install"); + assert.equal(typeof r.sizeMB, "number"); + assert.ok(r.sizeMB > 0); + assert.match(r.reason, /GB to download/); +}); + +test("a failed run still reports the tier's size", () => { + const r = runLocalModel("tts", { + specs: strongCpu, + which: ok, + exec: () => { + throw new Error("boom"); + }, + }); + + assert.equal(r.recommend, "install"); + assert.equal(typeof r.sizeMB, "number"); +}); diff --git a/skills/media-use/scripts/lib/ltx-video-provider.mjs b/skills/media-use/scripts/lib/ltx-video-provider.mjs index 9fb9a6755a..78aef41ac9 100644 --- a/skills/media-use/scripts/lib/ltx-video-provider.mjs +++ b/skills/media-use/scripts/lib/ltx-video-provider.mjs @@ -1,8 +1,8 @@ import { execFileSync } from "node:child_process"; -import { existsSync } from "node:fs"; +import { existsSync, unlinkSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { probeSpecs } from "./specs.mjs"; +import { describeDownload, probeSpecs } from "./specs.mjs"; import { buildArgv, selectModel, selectModelLadder } from "./local-models.mjs"; export async function ltxVideoGenerate( @@ -10,6 +10,7 @@ export async function ltxVideoGenerate( ctx, execFn = execFileSync, pathExists = existsSync, + unlinkFn = unlinkSync, ) { const specs = ctx?.specs || probeSpecs(); const ladder = selectModelLadder("videogen", specs, { preferTier: ctx?.preferTier }); @@ -21,6 +22,18 @@ export async function ltxVideoGenerate( return null; } + // Each attempt mints its own timestamped output path, so a partial artifact + // from a failed tier is orphaned rather than overwritten - and a lower tier + // then succeeding hides it. Discard it before demoting. Best-effort: a + // partial we cannot remove must never mask the real failure. + const discardPartial = (path) => { + try { + if (pathExists(path)) unlinkFn(path); + } catch { + // nothing actionable: the generate failure below is the real story + } + }; + // Walk the whole ladder, best tier first. A tier that cannot run on this // machine for a reason no spec check sees (runner off PATH, gated weights, an // OOM) demotes to the next fitting tier instead of failing local video gen @@ -32,7 +45,7 @@ export async function ltxVideoGenerate( execFn("which", [bin], { stdio: ["ignore", "ignore", "ignore"] }); } catch { console.error( - `media-use: local video gen not enabled (\`${bin}\` not on PATH). Install for free on-device LTX: ${model.install}`, + `media-use: local video gen not enabled (\`${bin}\` not on PATH). Install for free on-device LTX: ${model.install}. Heads up: ${model.id} ${describeDownload(model.sizeMB)}.`, ); continue; } @@ -54,6 +67,7 @@ export async function ltxVideoGenerate( stdio: ["ignore", "pipe", "pipe"], }); } catch (err) { + discardPartial(outPath); console.error( `media-use: local video gen (${model.id}) failed: ${err.stderr?.toString().trim().slice(-200) || err.message}`, ); diff --git a/skills/media-use/scripts/lib/ltx-video-provider.test.mjs b/skills/media-use/scripts/lib/ltx-video-provider.test.mjs index bbd7a0112c..2ad43f77ad 100644 --- a/skills/media-use/scripts/lib/ltx-video-provider.test.mjs +++ b/skills/media-use/scripts/lib/ltx-video-provider.test.mjs @@ -41,6 +41,8 @@ test("binary missing from PATH: prints the model install hint and falls through" assert.deepEqual(calls[0].slice(0, 2), ["which", ["ltx-2-mlx"]]); assert.equal(errors.length, 1); assert.match(errors[0], /git clone https:\/\/github\.com\/dgrauet\/ltx-2-mlx/); + // the install hint is the accept moment: say what the pull costs + assert.match(errors[0], /GB of weights to/); }); test("generate argv substitutes a spaced prompt after tokenizing and uses verified defaults", async () => { @@ -221,3 +223,94 @@ test("preferTier pins the attempt to one tier instead of demoting", async (t) => assert.equal(generated.length, 1, "pinned to large: no demotion to medium"); assert.match(generated[0], /dgrauet\/ltx-2\.3-mlx-q8/); }); + +// A failed attempt's temp path is minted per attempt (it carries a timestamp), +// so without cleanup a partial mp4 from a failed tier is orphaned rather than +// overwritten - and a lower tier then succeeding hides it. Partial video files +// are the expensive case, which is why this is pinned. +const outputOf = (argv) => argv[argv.indexOf("--output") + 1]; + +test("a failed attempt's partial output is discarded before demoting", async (t) => { + t.mock.method(console, "error", () => {}); + const unlinked = []; + const attempted = []; + const fakeExec = (...call) => { + if (!isGenerate(call)) return; + attempted.push(outputOf(call[1])); + if (call[1].includes("dgrauet/ltx-2.3-mlx-q8")) { + // OOM mid-write is one of the advertised demotion cases + const err = new Error("exit 1"); + err.stderr = "mlx.core.metal: out of memory"; + throw err; + } + }; + + const result = await ltxVideoGenerate( + "storm clouds", + { specs: bothTiersSpecs }, + fakeExec, + () => true, + (path) => unlinked.push(path), + ); + + assert.ok(result, "the medium tier still produced a video"); + assert.deepEqual(unlinked, [attempted[0]], "the failed large-tier partial is removed"); +}); + +test("every tier failing discards every partial, one per attempt", async (t) => { + t.mock.method(console, "error", () => {}); + const unlinked = []; + const attempted = []; + const fakeExec = (...call) => { + if (!isGenerate(call)) return; + attempted.push(outputOf(call[1])); + throw new Error("mlx out of memory"); + }; + + const result = await ltxVideoGenerate( + "storm clouds", + { specs: bothTiersSpecs }, + fakeExec, + () => true, + (path) => unlinked.push(path), + ); + + assert.equal(result, null); + assert.equal(attempted.length, 2, "both tiers attempted"); + assert.deepEqual(unlinked, attempted, "nothing is left behind on the all-fail path"); +}); + +test("a successful generation is never discarded", async () => { + const unlinked = []; + + const result = await ltxVideoGenerate( + "storm clouds", + { specs: bothTiersSpecs }, + () => {}, + () => true, + (path) => unlinked.push(path), + ); + + assert.ok(result); + assert.deepEqual(unlinked, [], "the returned artifact must survive"); +}); + +test("an unremovable partial does not mask the generate failure", async (t) => { + t.mock.method(console, "error", () => {}); + const fakeExec = (...call) => { + if (isGenerate(call)) throw new Error("mlx out of memory"); + }; + + const result = await ltxVideoGenerate( + "storm clouds", + { specs: bothTiersSpecs }, + fakeExec, + () => true, + () => { + throw new Error("EPERM: operation not permitted"); + }, + ); + + // cleanup is best-effort: a partial we cannot delete must not become the error + assert.equal(result, null); +}); diff --git a/skills/media-use/scripts/lib/mflux-provider.mjs b/skills/media-use/scripts/lib/mflux-provider.mjs index cc7157a07f..af5511ad85 100644 --- a/skills/media-use/scripts/lib/mflux-provider.mjs +++ b/skills/media-use/scripts/lib/mflux-provider.mjs @@ -1,8 +1,8 @@ import { execFileSync } from "node:child_process"; -import { existsSync } from "node:fs"; +import { existsSync, unlinkSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { probeSpecs } from "./specs.mjs"; +import { describeDownload, probeSpecs } from "./specs.mjs"; import { buildArgv, selectModelLadder } from "./local-models.mjs"; // Local image generation via mflux (FLUX-on-MLX), the Mac-native runner. @@ -33,11 +33,24 @@ export async function mfluxImageGenerate( ctx, execFn = execFileSync, pathExists = existsSync, + unlinkFn = unlinkSync, ) { const specs = ctx?.specs || probeSpecs(); const ladder = selectModelLadder("imagegen", specs, { preferTier: ctx?.preferTier }); if (!ladder.length) return null; // no local model fits -> codex upsell/fallback + // Each attempt mints its own timestamped output path, so a partial artifact + // from a failed tier is orphaned rather than overwritten - and a lower tier + // then succeeding hides it. Discard it before demoting. Best-effort: a + // partial we cannot remove must never mask the real failure. + const discardPartial = (path) => { + try { + if (pathExists(path)) unlinkFn(path); + } catch { + // nothing actionable: the generate failure below is the real story + } + }; + // Best tier first, demoting past any tier that cannot run here (runner off // PATH, a snapshot that won't download, an OOM) rather than failing local // image gen outright. Every demotion is reported, so a quietly smaller model @@ -51,7 +64,7 @@ export async function mfluxImageGenerate( execFn("which", [bin], { stdio: ["ignore", "ignore", "ignore"] }); } catch { console.error( - `media-use: local image gen not enabled (\`${bin}\` not on PATH). Install for free on-device FLUX: ${model.install}`, + `media-use: local image gen not enabled (\`${bin}\` not on PATH). Install for free on-device FLUX: ${model.install}. Heads up: ${model.id} ${describeDownload(model.sizeMB)}.`, ); continue; } @@ -88,6 +101,7 @@ export async function mfluxImageGenerate( stdio: ["ignore", "pipe", "pipe"], }); } catch (err) { + discardPartial(outPath); console.error( `media-use: local image gen (${model.id}) failed: ${err.stderr?.toString().trim().slice(-200) || err.message}`, ); diff --git a/skills/media-use/scripts/lib/mflux-provider.test.mjs b/skills/media-use/scripts/lib/mflux-provider.test.mjs index 9bb89c00a5..0ca51904bc 100644 --- a/skills/media-use/scripts/lib/mflux-provider.test.mjs +++ b/skills/media-use/scripts/lib/mflux-provider.test.mjs @@ -112,4 +112,63 @@ test("runner missing from PATH reports the install hint per tier and returns nul assert.equal(result, null); assert.equal(errors.length, 2, "both fitting tiers reported"); assert.match(errors[0], /uv pip install mflux/); + // each hint states that tier's download cost before the user commits + assert.match(errors[0], /GB of weights to/); + assert.match(errors[1], /GB of weights to/); +}); + +// Same per-attempt temp path, same orphaning risk as the LTX provider: a +// partial png from a failed tier must not survive a lower tier succeeding. +const outputOf = (argv) => argv[argv.indexOf("--output") + 1]; +const attemptedOutputs = (calls) => + calls.filter(([bin]) => bin !== "which" && bin !== "hf").map(([, argv]) => outputOf(argv)); + +test("a failed attempt's partial output is discarded before demoting", async (t) => { + t.mock.method(console, "error", () => {}); + const unlinked = []; + const { calls, exec } = stubExec({ failGenerateFor: ["flux2-klein-4b"] }); + + const result = await mfluxImageGenerate( + "a red bicycle", + { specs: bothTiersSpecs }, + exec, + () => true, + (path) => unlinked.push(path), + ); + + assert.ok(result, "the schnell tier still produced an image"); + assert.deepEqual(unlinked, [attemptedOutputs(calls)[0]], "the failed klein partial is removed"); +}); + +test("every tier failing discards every partial, one per attempt", async (t) => { + t.mock.method(console, "error", () => {}); + const unlinked = []; + const { calls, exec } = stubExec({ failGenerateFor: ["flux2-klein-4b", "schnell"] }); + + const result = await mfluxImageGenerate( + "a red bicycle", + { specs: bothTiersSpecs }, + exec, + () => true, + (path) => unlinked.push(path), + ); + + assert.equal(result, null); + assert.deepEqual(unlinked, attemptedOutputs(calls), "nothing is left behind"); +}); + +test("a successful generation is never discarded", async () => { + const unlinked = []; + const { exec } = stubExec(); + + const result = await mfluxImageGenerate( + "a red bicycle", + { specs: bothTiersSpecs }, + exec, + () => true, + (path) => unlinked.push(path), + ); + + assert.ok(result); + assert.deepEqual(unlinked, [], "the returned artifact must survive"); }); diff --git a/skills/media-use/scripts/lib/specs.mjs b/skills/media-use/scripts/lib/specs.mjs index 831e4eb170..3d1cef24ff 100644 --- a/skills/media-use/scripts/lib/specs.mjs +++ b/skills/media-use/scripts/lib/specs.mjs @@ -9,6 +9,8 @@ // stdout as a string, or throws / returns null on failure. import os from "node:os"; +import { statfsSync } from "node:fs"; +import { dirname, join } from "node:path"; import { execSync } from "node:child_process"; function defaultExec(cmd) { @@ -79,3 +81,52 @@ export function probeSpecs({ osMod = os, exec = defaultExec } = {}) { gpu: detectGpu(platform, arch, ramMB, exec), }; } + +// Where Hugging Face actually puts downloaded weights. A free-space check +// against cwd measures the wrong filesystem, so the disk question has to be +// asked about this directory. Precedence follows huggingface_hub's own order. +export function weightsCacheDir({ env = process.env, osMod = os } = {}) { + if (env.HF_HUB_CACHE) return env.HF_HUB_CACHE; + if (env.HUGGINGFACE_HUB_CACHE) return env.HUGGINGFACE_HUB_CACHE; + if (env.HF_HOME) return join(env.HF_HOME, "hub"); + return join(osMod.homedir(), ".cache", "huggingface", "hub"); +} + +// Free space on the filesystem that will hold the weights. The cache dir +// usually does not exist until the first download and statfs throws on a +// missing path, so walk up to the deepest ancestor that does exist. Returns +// null when even the root cannot be read, so callers can say "unknown" instead +// of implying zero and scaring someone off a download that would have worked. +export function freeSpaceMB(dir, statfsFn = statfsSync) { + let path = dir; + for (;;) { + try { + const { bavail, bsize } = statfsFn(path); + return (bavail * bsize) / 1e6; + } catch { + const parent = dirname(path); + if (parent === path) return null; + path = parent; + } + } +} + +// One line the user reads BEFORE agreeing to a pull that can be tens of GB. +// Always names the size and where it lands. When it will not fit we say so +// plainly rather than withholding the tier: a machine that could free up space +// should still be told the tier exists. `statfsFn` / `env` / `osMod` are +// injectable for tests. +export function describeDownload( + sizeMB, + { statfsFn = statfsSync, env = process.env, osMod = os } = {}, +) { + const dir = weightsCacheDir({ env, osMod }); + const gb = (mb) => (mb / 1000).toFixed(1); + const head = `downloads ~${gb(sizeMB)}GB of weights to ${dir}`; + const free = freeSpaceMB(dir, statfsFn); + if (free == null) return `${head} (free space unknown)`; + if (free < sizeMB) { + return `${head}, but only ${gb(free)}GB is free there, so it will NOT fit as-is`; + } + return `${head} (${gb(free)}GB free there)`; +} diff --git a/skills/media-use/scripts/lib/specs.test.mjs b/skills/media-use/scripts/lib/specs.test.mjs index 1c9172b318..ff692092ad 100644 --- a/skills/media-use/scripts/lib/specs.test.mjs +++ b/skills/media-use/scripts/lib/specs.test.mjs @@ -1,6 +1,6 @@ import { strict as assert } from "node:assert"; import { test } from "node:test"; -import { probeSpecs } from "./specs.mjs"; +import { describeDownload, freeSpaceMB, probeSpecs, weightsCacheDir } from "./specs.mjs"; // Fake os module + exec so the probe is deterministic across CI machines. const fakeOs = (over = {}) => ({ @@ -73,3 +73,85 @@ test("no GPU when nvidia-smi is absent / fails", () => { assert.equal(s.gpu.present, false); assert.equal(s.gpu.vramMB, 0); }); + +// --- download disclosure: what the user is agreeing to before the pull --- + +const fakeHome = { homedir: () => "/home/tester" }; +// statfs reports blocks, not bytes: bavail * bsize. 1 MB blocks keep the sums +// readable, and mirror the real struct's shape. +const fakeStatfs = (freeMB, existsOnly) => (path) => { + if (existsOnly && path !== existsOnly) throw new Error(`ENOENT: ${path}`); + return { bavail: freeMB, bsize: 1e6 }; +}; + +test("weightsCacheDir follows huggingface_hub's precedence", () => { + assert.equal( + weightsCacheDir({ env: {}, osMod: fakeHome }), + "/home/tester/.cache/huggingface/hub", + ); + assert.equal(weightsCacheDir({ env: { HF_HOME: "/data/hf" }, osMod: fakeHome }), "/data/hf/hub"); + assert.equal( + weightsCacheDir({ env: { HF_HOME: "/data/hf", HUGGINGFACE_HUB_CACHE: "/c" }, osMod: fakeHome }), + "/c", + "HUGGINGFACE_HUB_CACHE outranks HF_HOME", + ); + assert.equal( + weightsCacheDir({ env: { HF_HUB_CACHE: "/a", HUGGINGFACE_HUB_CACHE: "/c" }, osMod: fakeHome }), + "/a", + "HF_HUB_CACHE wins outright", + ); +}); + +test("freeSpaceMB walks up to the deepest existing ancestor", () => { + // the cache dir does not exist until the first download, and statfs throws + // on a missing path - so the answer has to come from an ancestor + const statfs = fakeStatfs(4096, "/home"); + assert.equal(freeSpaceMB("/home/tester/.cache/huggingface/hub", statfs), 4096); +}); + +test("freeSpaceMB reports null rather than zero when nothing can be read", () => { + assert.equal( + freeSpaceMB("/home/tester/.cache", () => { + throw new Error("EACCES"); + }), + null, + "unknown must not be reported as no-space", + ); +}); + +test("describeDownload names the size and where it lands", () => { + const msg = describeDownload(87500, { + statfsFn: fakeStatfs(200000), + env: {}, + osMod: fakeHome, + }); + assert.match(msg, /~87\.5GB/); + assert.match(msg, /\/home\/tester\/\.cache\/huggingface\/hub/); + assert.match(msg, /200\.0GB free/); + assert.equal(/NOT fit/.test(msg), false, "it fits, so no warning"); +}); + +test("describeDownload says plainly when the weights will not fit", () => { + const msg = describeDownload(87500, { + statfsFn: fakeStatfs(14000), + env: {}, + osMod: fakeHome, + }); + assert.match(msg, /only 14\.0GB is free there/); + assert.match(msg, /will NOT fit as-is/); + // the tier is still described, not withheld: a machine that could free up + // space should know the tier exists + assert.match(msg, /~87\.5GB/); +}); + +test("describeDownload admits when free space is unknown", () => { + const msg = describeDownload(87500, { + statfsFn: () => { + throw new Error("EACCES"); + }, + env: {}, + osMod: fakeHome, + }); + assert.match(msg, /free space unknown/); + assert.equal(/NOT fit/.test(msg), false, "unknown is not a refusal"); +});