fix: pipeline_state.js -- deep-merge queued saves, close a load race
Review found the save() queue's pendingBody merge was shallow: two queued
edits that both touch the top-level "inputs" key (two different slots, or
even two fields on the same slot from models.js's per-call-fresh objects)
had the second's inputs object silently REPLACE the first's rather than
combine -- the earlier edit vanished with no error, the field just
reverted on the next render. engine_settings.js's own calls happened to be
immune by accident (one long-lived mutated `values` object, so the last
queued snapshot already contained every earlier edit) -- that immunity
was never a property of save() itself, and models.js's fresh-object-per-
call shape hit it immediately. Fixed with a one-level-deeper merge for
"inputs"/"values" specifically. Verified live via two real race
reproductions run through the browser console: two edits on different
slots queued with no await between them, and two edits on the SAME slot --
both survive now, confirmed against the actual saved graph state.
Also closed a load race: ensureLoaded()'s only guard was `slots !== null`,
which doesn't cover the window while a first load is still in flight -- two
panels mounting close together could both start a fetch. Now every caller
awaits the same in-flight promise, so exactly one fetch ever goes out per
session.
Documented (not fixed, deliberately) that Models & Pipeline's node-level
pin target is currently unreachable: pinTarget() only ever returns a
group-level pin today, since there's no "currently open node" concept in
this flat slot list yet -- the openNode/ctx.sub("node:...") plumbing stays
for whatever old pin data might still reference it, but nothing produces
a fresh one. Real gap against v4's per-node pinning, not a regression.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
770d58a315
commit
c6559765b7
@@ -242,6 +242,14 @@
|
||||
iconBg: "linear-gradient(180deg,#b18cff,#7a4fd0)",
|
||||
icon: '<svg viewBox="0 0 16 16" width="13" height="13" fill="none" stroke="#fff" stroke-width="1.4" stroke-linejoin="round"><path d="M8 1.8 14 5v6l-6 3.2L2 11V5l6-3.2z"/><path d="M2 5l6 3 6-3M8 8v6.2"/></svg>',
|
||||
mount,
|
||||
// Group-level pinning only, deliberately -- v4 could pin ONE node (settings_window.js's
|
||||
// own comment: "the node page is the thing that takes several clicks to reach"), which
|
||||
// needed a concept of "the currently open node" this flat slot list doesn't have yet
|
||||
// (nothing here drills into a single slot the way the old per-role node page did).
|
||||
// `openNodeId`/`ModelsModal.openNode` below still work if something hands them a real
|
||||
// slot id -- pinned_buttons.js still calls them -- but nothing on this screen currently
|
||||
// PRODUCES a node-level pin to begin with, so a per-node pin from before this rewrite
|
||||
// is the only way this path is reached today. Group-level is the honest current target.
|
||||
pinTarget: () => (group
|
||||
? { kind: "section", id: "models", sub: group, label: `Models ▸ ${group}` }
|
||||
: null),
|
||||
|
||||
@@ -48,21 +48,62 @@
|
||||
return merged;
|
||||
}
|
||||
|
||||
// Two panels can mount close enough together that both call this before
|
||||
// the first fetch settles (`slots` only becomes non-null at the end of a
|
||||
// load, so a bare `if (slots !== null) return` doesn't cover the window
|
||||
// while one is already in flight). Every caller awaits the SAME in-flight
|
||||
// promise instead, so exactly one fetch ever goes out per session, however
|
||||
// many panels ask.
|
||||
let loadPromise = null;
|
||||
|
||||
async function ensureLoaded() {
|
||||
if (slots !== null) return; // session-wide: every consumer shares this one load
|
||||
if (loadPromise) return loadPromise;
|
||||
loading = true; loadError = null;
|
||||
try {
|
||||
const [manifest, pipe] = await Promise.all([API.modules(), API.pipeline()]);
|
||||
modulesById = {};
|
||||
(manifest.modules || []).forEach((m) => { modulesById[m.id] = m; });
|
||||
slots = pipe.slots || [];
|
||||
incomplete = pipe.incomplete || [];
|
||||
refused = pipe.refused || [];
|
||||
queueable = !!pipe.queueable;
|
||||
} catch (e) {
|
||||
loadError = e && e.message ? e.message : String(e);
|
||||
}
|
||||
loading = false;
|
||||
loadPromise = (async () => {
|
||||
try {
|
||||
const [manifest, pipe] = await Promise.all([API.modules(), API.pipeline()]);
|
||||
modulesById = {};
|
||||
(manifest.modules || []).forEach((m) => { modulesById[m.id] = m; });
|
||||
slots = pipe.slots || [];
|
||||
incomplete = pipe.incomplete || [];
|
||||
refused = pipe.refused || [];
|
||||
queueable = !!pipe.queueable;
|
||||
} catch (e) {
|
||||
loadError = e && e.message ? e.message : String(e);
|
||||
}
|
||||
loading = false;
|
||||
loadPromise = null;
|
||||
})();
|
||||
return loadPromise;
|
||||
}
|
||||
|
||||
// `save()`'s body is one level deeper than a shallow spread can merge
|
||||
// correctly: `inputs`/`values` are both {outerId: {innerKey: value}}, and
|
||||
// two queued saves that both touch `inputs` (two different slots, or even
|
||||
// two fields on the SAME slot from models.js's per-call-fresh objects) had
|
||||
// the second's `inputs` object silently REPLACE the first's rather than
|
||||
// combine with it -- the earlier edit vanished with no error. Merges one
|
||||
// level deeper than Object.assign/spread does, for exactly the two keys
|
||||
// that are ever shaped this way.
|
||||
function mergeBodies(a, b) {
|
||||
const out = { ...(a || {}) };
|
||||
Object.entries(b || {}).forEach(([key, val]) => {
|
||||
const prior = out[key];
|
||||
const bothPlainObjects = val && typeof val === "object" && !Array.isArray(val)
|
||||
&& prior && typeof prior === "object" && !Array.isArray(prior);
|
||||
if (!bothPlainObjects) { out[key] = val; return; }
|
||||
const merged = { ...prior };
|
||||
Object.entries(val).forEach(([innerId, inner]) => {
|
||||
const priorInner = merged[innerId];
|
||||
merged[innerId] = (inner && typeof inner === "object" && !Array.isArray(inner)
|
||||
&& priorInner && typeof priorInner === "object" && !Array.isArray(priorInner))
|
||||
? { ...priorInner, ...inner }
|
||||
: inner;
|
||||
});
|
||||
out[key] = merged;
|
||||
});
|
||||
return out;
|
||||
}
|
||||
|
||||
// One shared save queue for every caller. `body` is layered onto the
|
||||
@@ -74,7 +115,7 @@
|
||||
// retry-after-in-flight shape exists (a same-panel double-edit dropped
|
||||
// one edit before this).
|
||||
async function save(body) {
|
||||
pendingBody = { ...(pendingBody || {}), ...body };
|
||||
pendingBody = mergeBodies(pendingBody, body);
|
||||
if (saving) { pending = true; return; }
|
||||
saving = true;
|
||||
do {
|
||||
|
||||
Reference in New Issue
Block a user