fix: reference-wiring silent drops/overwrites, found by adversarial review
wireReferences() (new, extracted from boot.js's queueInputs) now reports an unwired count instead of silently losing a reference when: the pipeline has fewer reference slots than the scene has references, two references' roles resolve to the same slot or downstream destination, or a reference's slot was already claimed by the scene's source-image assignment. boot.js toasts whenever unwired > 0, worded to cover both the capacity and collision cases. None of these are reachable through the one shipped preset (H3's R2V, one role per slot) -- all are latent contract violations a hand-edited or future preset could trigger. 10 new tests in reference_wiring.test.js.
This commit is contained in:
+19
-19
@@ -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");
|
||||
|
||||
@@ -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 };
|
||||
}
|
||||
@@ -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);
|
||||
});
|
||||
Reference in New Issue
Block a user