diff --git a/app/boot.js b/app/boot.js index 8fce6c39..96ef6d83 100644 --- a/app/boot.js +++ b/app/boot.js @@ -28,6 +28,7 @@ import { mount as mountLog } from "./shell/logwindow.js"; import { mount as mountTemp } from "./shell/tempfiles.js"; import { mount as mountShortcuts } from "./shell/shortcuts.js"; import { createSettingsWindow } from "./shell/settings_window.js"; +import { wireReferences } from "./shell/reference_wiring.js"; const root = document.querySelector("#app"); @@ -390,25 +391,24 @@ async function start() { const source = slotForRole("assets.source_image"); if (source) raw[source.id] = { ...raw[source.id], media_id: scene.source_image || "" }; - const refs = scene.references || []; - refs.forEach((mediaId, i) => { - const at = `assets.reference_${i + 1}`; - const slot = (slots || []).find((s) => (s.roles || []).some((r) => r.at === at)); - if (!slot) return; - raw[slot.id] = { ...raw[slot.id], media_id: mediaId }; - // Only a reference the scene actually has gets wired into whatever - // reads it -- an unwired slot, not an empty image, is what "not in - // use" has to mean to an autogrow reference input; see - // modules/models/minimax_h3/pipeline.py's own docstring for why. The - // wire's target is the role's own data (`wireTo`), never a slot id - // this file would otherwise have to know: the preset declares where - // its own reference lands, the same way it declares everything else. - const role = slot.roles.find((r) => r.at === at); - if (role && role.wireTo) { - raw[role.wireTo.slot] = { ...raw[role.wireTo.slot], - [role.wireTo.input]: [slot.id, 0] }; - } - }); + const { overrides, unwired } = wireReferences( + scene.references || [], slots || [], source ? [source.id] : []); + for (const [id, fields] of Object.entries(overrides)) { + raw[id] = { ...raw[id], ...fields }; + } + // `unwired` covers two different causes -- the pipeline has fewer + // reference slots than the scene has references (H3's R2V caps at 4 + // today), or a malformed preset points two references at the same + // slot/destination -- but from here they look the same, and saying + // "no slot" for the second would send someone hunting for a bigger + // pipeline instead of the real, preset-authoring cause. Word it as + // what is actually and always true of both: the reference did not + // make it into this run. + if (unwired > 0) { + composer.toast.warn({ text: unwired === 1 + ? "1 reference could not be wired into this pipeline and will not be used." + : `${unwired} references could not be wired into this pipeline and will not be used.` }); + } } const field = prompts && prompts.fields.find((f) => f.at === "generation.prompt"); diff --git a/app/shell/reference_wiring.js b/app/shell/reference_wiring.js new file mode 100644 index 00000000..e40be045 --- /dev/null +++ b/app/shell/reference_wiring.js @@ -0,0 +1,49 @@ +// Turns a scene's picked reference-media ids into slot overrides for whatever +// pipeline is current -- pulled out of boot.js so it can be tested without +// booting the whole app (boot.js runs start() on import). + +/** + * wireReferences(mediaIds, slots, reservedSlotIds) -> { overrides, unwired } + * + * `overrides` is keyed by slot id, merge these into the raw input overrides + * sent with a run. `unwired` counts references that did not make it into the + * run -- the pipeline has fewer "assets.reference_N" roles than the scene has + * references (e.g. added under a bigger preset, then the preset was swapped + * for a smaller one), two references' roles resolve to the same destination, + * or a reference's slot is one the caller already assigned to something else + * (`reservedSlotIds` -- e.g. a slot that also carries the scene's + * `assets.source_image` role). Any of these silently overwriting or dropping + * a reference would look like it was used; the caller is the one place that + * knows this happened and must say so. + */ +export function wireReferences(mediaIds, slots, reservedSlotIds) { + const pool = slots || []; + const overrides = {}; + const claimed = new Set(reservedSlotIds || []); + let unwired = 0; + mediaIds.forEach((mediaId, i) => { + const at = `assets.reference_${i + 1}`; + const slot = pool.find((s) => (s.roles || []).some((r) => r.at === at)); + if (!slot) { unwired += 1; return; } + // The wire's target is the role's own data (`wireTo`), never a slot id + // this file would otherwise have to know: the preset declares where its + // own reference lands, the same way it declares everything else. + const role = slot.roles.find((r) => r.at === at); + // A destination -- the slot's own media_id input, or wherever `wireTo` + // points -- can hold one value. Two references claiming the SAME slot + // (a preset with two reference roles on one slot) or the SAME wireTo + // target (two different slots both wired into one downstream input) are + // the same failure either way: the second silently overwrites the first + // unless it is counted unwired instead. + const dest = role && role.wireTo ? `${role.wireTo.slot}.${role.wireTo.input}` : slot.id; + if (claimed.has(slot.id) || claimed.has(dest)) { unwired += 1; return; } + claimed.add(slot.id); + claimed.add(dest); + overrides[slot.id] = { ...overrides[slot.id], media_id: mediaId }; + if (role && role.wireTo) { + overrides[role.wireTo.slot] = { ...overrides[role.wireTo.slot], + [role.wireTo.input]: [slot.id, 0] }; + } + }); + return { overrides, unwired }; +} diff --git a/app/shell/tests/reference_wiring.test.js b/app/shell/tests/reference_wiring.test.js new file mode 100644 index 00000000..bfe905e0 --- /dev/null +++ b/app/shell/tests/reference_wiring.test.js @@ -0,0 +1,89 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { wireReferences } from "../reference_wiring.js"; + +function refSlot(n) { + return { id: `ref_source_${n}`, roles: [{ at: `assets.reference_${n}`, + input: "media_id", wireTo: { slot: "r2v", input: `ref_images.ref_image_${n - 1}` } }] }; +} + +test("each picked reference is wired to its slot and to r2v", () => { + const slots = [refSlot(1), refSlot(2)]; + const { overrides, unwired } = wireReferences(["aaa111111111", "bbb222222222"], slots); + assert.equal(unwired, 0); + assert.equal(overrides.ref_source_1.media_id, "aaa111111111"); + assert.deepEqual(overrides.r2v["ref_images.ref_image_0"], ["ref_source_1", 0]); + assert.deepEqual(overrides.r2v["ref_images.ref_image_1"], ["ref_source_2", 0]); +}); + +test("a reference beyond the pipeline's slots is counted, not silently dropped", () => { + // This is the bug an adversarial review caught: a scene can carry more + // references than the CURRENT pipeline offers slots for (e.g. added under + // a bigger preset, then the preset was swapped for a smaller one). The old + // code in boot.js just returned from the forEach iteration and moved on -- + // no override, no count, no way for the caller to tell the user anything + // was left out. + const slots = [refSlot(1)]; // only one slot offered + const { overrides, unwired } = wireReferences( + ["aaa111111111", "bbb222222222", "ccc333333333"], slots); + assert.equal(unwired, 2); + assert.equal(overrides.ref_source_1.media_id, "aaa111111111"); + assert.equal(Object.keys(overrides).length, 2); // ref_source_1 + r2v, nothing else +}); + +test("no references and no slots wires nothing and reports no shortfall", () => { + const { overrides, unwired } = wireReferences([], []); + assert.deepEqual(overrides, {}); + assert.equal(unwired, 0); +}); + +test("slots not yet loaded (null, boot.js's own initial state) is treated as no slots, not a crash", () => { + const { overrides, unwired } = wireReferences(["aaa111111111"], null); + assert.deepEqual(overrides, {}); + assert.equal(unwired, 1); +}); + +test("a slot whose roles claim two reference numbers does not let the second silently overwrite the first", () => { + // Not producible by the shipped H3 preset (one role per slot), but nothing + // stops a future preset's roles array from doing this -- the function's own + // contract ("no reference silently dropped") has to hold for the input + // shape, not just the one preset that happens to exist today. + const combo = { id: "combo_slot", roles: [ + { at: "assets.reference_1", wireTo: { slot: "r2v", input: "ref_images.ref_image_0" } }, + { at: "assets.reference_2", wireTo: { slot: "r2v", input: "ref_images.ref_image_1" } }, + ] }; + const { overrides, unwired } = wireReferences(["AAA", "BBB"], [combo]); + assert.equal(overrides.combo_slot.media_id, "AAA"); + assert.equal(unwired, 1); +}); + +test("two different slots wired to the same downstream input do not let the second overwrite the first", () => { + // Same failure as the combo-slot case above, reached a different way: two + // separate reference slots whose `wireTo` both point at one destination + // (e.g. a copy-pasted preset). The destination can hold one value. + const slots = [ + { id: "ref_source_1", roles: [{ at: "assets.reference_1", + wireTo: { slot: "r2v", input: "ref_images.ref_image_0" } }] }, + { id: "ref_source_2", roles: [{ at: "assets.reference_2", + wireTo: { slot: "r2v", input: "ref_images.ref_image_0" } }] }, + ]; + const { overrides, unwired } = wireReferences(["AAA", "BBB"], slots); + assert.deepEqual(overrides.r2v["ref_images.ref_image_0"], ["ref_source_1", 0]); + assert.equal(overrides.ref_source_1.media_id, "AAA"); + assert.equal(overrides.ref_source_2, undefined); + assert.equal(unwired, 1); +}); + +test("a slot the caller already assigned (e.g. the source-image slot) is not silently overwritten by a reference", () => { + // boot.js sets the source-image slot's media_id BEFORE calling + // wireReferences() -- if a preset ever reused that same slot id for a + // reference role too, the reference would clobber the source image with + // no toast, since wireReferences() has no visibility into that earlier + // write on its own. The caller passes it in as reserved instead. + const slots = [refSlot(1)]; + const { overrides, unwired } = wireReferences( + ["AAA"], slots, ["ref_source_1"]); + assert.equal(overrides.ref_source_1, undefined); + assert.equal(unwired, 1); +});