fix(inspect): abortable, stepped local reads with a busy state

The wait for the node table now ends with the selection's signal. A
local read resolves and projects in bounded steps with a turn of the
event loop between them, and the inspector carries aria-busy while it
loads. A budget test covers maximal 2 MiB graphs and checks the stepped
read is byte-identical to one synchronous build. The D117 wrap rule now
targets only the summary button, and TESTING.md scopes the restart line
to PRs L1 and L2.
This commit is contained in:
Vito Sansevero
2026-10-22 19:17:53 -07:00
parent 29e2bfd9ef
commit b4aa25a3e7
9 changed files with 214 additions and 15 deletions
+16 -2
View File
@@ -1588,7 +1588,8 @@ AVI/WMV preview only), chosen or dropped, previews inline. The heading's one act
the shared viewer with the full inspector on a record read entirely in this browser
(`web/js/workspace/inspect.mjs`, `local-session.mjs`, `local-inspection.mjs` and the L1
readers `png-metadata.js`, `local-video.mjs`, `local-projection.mjs`). Nothing is
uploaded, imported or stored. Only HTML and JS change, so no ComfyUI restart is needed.
uploaded, imported or stored. PRs L1 and L2 change only HTML and JS, so they need no
ComfyUI restart; PR L3 does (below).
Local roles (#249 PR L3, decision LI-1): the prompts and resolution of a local record are
the server inspector's for the same records. `web/js/workspace/local-resolver.mjs` (with
@@ -1610,7 +1611,20 @@ vector (prompts, resolution and facts), the golden and selection record sets
bound and 1,200 seeded fuzz cases; `tests/fixtures/local-inspect/resolver-vectors.json`
holds the committed answers the Node unit test reads. The browser spec `chooser shows the
Images roles and resolution for every parity PNG` compares the rail with Images for all
25 committed parity PNGs. The static route change needs a ComfyUI restart.
25 committed parity PNGs. The static route change and the resolver fixes (Python) need a
ComfyUI restart.
Main thread (M4): the resolver runs on the page's thread, not a worker. On a desktop
(Node 26) `resolveLocalEvidence` on maximal 2 MiB graphs (a 19,990-sampler chain, 6,600
independent samplers, a consumer-dense scan, a 100,000-node PromptManager workflow) took
130–455 ms; the server took 80–905 ms on the same inputs. `localFileAdapter.read` runs in
bounded steps (read, table, parse and resolve, project, fit) with a turn of the event
loop between them, every wait ends with the selection's signal (a stalled table request
included), and the inspector carries `aria-busy="true"` with "Loading file details…"
until the answer lands. `tests/js/workspace-local-inspection.test.js` pins a 2,500 ms
budget per step on maximal independent-sampler and stage-chain graphs, checks that the
stepped read answers byte for byte as one synchronous build, and that an abort at any
step ends the read.
Bounds (the server's own budgets, ported by PR L1; see "Follow-up: local Inspect parity"
under "Chronological Library history" for the known divergences):
+2 -1
View File
@@ -240,8 +240,9 @@ test('viewer details hold provenance, raw records and downloads',async({page,req
// The panel content without the cover, D117's Copy settings summary after the facts, then the two disclosures,
// then the pair send and history buttons (D20).
expect(await viewerStructure(details)).toEqual({outer:['div','button.send','button'],
inner:['div.panel-heading','p.small','div.tags','section.prompt-section.positive','section.prompt-section.negative','div.tags','dl.meta-grid','button','details','details']});
inner:['div.panel-heading','p.small','div.tags','section.prompt-section.positive','section.prompt-section.negative','div.tags','dl.meta-grid','button.settings-summary','details','details']});
await expect(details.locator(':scope > div > button')).toHaveText('Copy settings summary');
await expect(details.locator(':scope > div > button.settings-summary')).toHaveCSS('white-space','normal');
await expect(details.locator('.detail-cover,.detail-inner,.detail-footer')).toHaveCount(0);
await expect(details.locator('.panel-heading h2')).toHaveText(POSITIVE_TITLE);await expect(details.locator('.panel-heading .chip')).toHaveText('PNG');
await expect(details.locator(':scope > div > p.small')).toHaveText('complete.png');
+1 -1
View File
@@ -119,7 +119,7 @@ async function plainViewerChips(page) {
});
const summary=document.querySelector('#viewer-details [data-action="download-workflow"]').cloneNode(false);
for(const key of Object.keys(summary.dataset))delete summary.dataset[key];
summary.textContent='Copy settings summary';
summary.textContent='Copy settings summary';summary.classList.add('settings-summary');
document.querySelector('#viewer-details > div > dl.meta-grid').after(summary);window.__summaryButton=summary;
});
await settle(page);
+30
View File
@@ -303,6 +303,8 @@ test('Copy settings summary follows the facts in the full inspector only', async
const inner = full.button.parent, index = inner.children.indexOf(full.button);
assert.equal(inner.children[index - 1].tagName, 'dl', 'it follows the six facts');
assert.equal(inner.children[index + 1].tagName, 'details', 'File & provenance follows it');
// M7: the D117 wrap rule targets this button by its class, never any button of a details block.
assert.equal(full.button.className, 'settings-summary');
} finally { full.done(); }
const panel = renderSummary(record(), {variant:'panel'});
try { assert.equal(panel.button, null, 'the Images side panel keeps the concept footer'); } finally { panel.done(); }
@@ -333,3 +335,31 @@ test('Copy settings summary without a clipboard opens the select-to-copy fallbac
} finally { view.done(); }
}
});
test('the inspector is busy while a read is loading, then not', async () => {
const f = mountedDom(), scope = createScope();
const view = mountFileInspector(f.container, {sender:{open() {}}, parentScope:scope, variant:'full'});
try {
view.setState({loading:true, error:false, record:null});
assert.equal(f.container.getAttribute('aria-busy'), 'true');
assert.ok(f.walk(f.container, n => n.textContent === 'Loading file details…').length);
view.setState({loading:false, error:false, record:record()});
assert.equal(f.container.getAttribute('aria-busy'), null);
view.setState({loading:true, error:false, record:null});
view.setState({loading:false, error:true, record:null});
assert.equal(f.container.getAttribute('aria-busy'), null);
view.setState({loading:true, error:false, record:null});
} finally {
view.dispose();
assert.equal(f.container.getAttribute('aria-busy'), null, 'dispose clears the busy state');
scope.dispose(); f.restore();
}
});
test('the D117 wrap rule targets only the settings summary button', async () => {
const fs = await import('node:fs');
const css = fs.readFileSync(new URL('../../web/js/workspace/workspace.css', import.meta.url), 'utf8');
const rules = css.split('\n').filter(line => line.includes('D117'));
assert.deepEqual(rules.map(line => line.trim()), ['.ca-tools #viewer-details .settings-summary { white-space: normal; /* D117 */ }']);
assert.doesNotMatch(css, /#viewer-details > div > button/);
});
+120
View File
@@ -23,8 +23,11 @@ globalThis.ComfyMetadata = ComfyMetadata;
// The page's API client and the node table it serves (local-semantics.mjs fetches it once).
const SEMANTICS = fs.readFileSync(path.join(here, "..", "..", "web", "js", "inspection-semantics.json"), "utf8");
globalThis.window = { ApiClient: { resolveUrl: (value) => value } };
// A test can hold the table request (`tableGate`) to stand for a slow or stalled proxy.
let tableGate = null;
globalThis.fetch = async (url) => {
if (url !== "/prompt_manager/js/inspection-semantics.json") throw new Error(`unexpected request ${url}`);
if (tableGate) await tableGate;
return new Response(SEMANTICS, { status: 200 });
};
const m = await import("../../web/js/workspace/local-inspection.mjs").catch((error) => {
@@ -66,6 +69,37 @@ test("contract", () => {
assert.ok(Object.isFrozen(m.localFileAdapter));
});
// M3: the first read waits for the page's one table request; an aborted, replaced or cleared selection must not.
test("a read waiting for the node table ends with its selection", async () => {
let release;
tableGate = new Promise((resolve) => { release = resolve; });
try {
const file = pngFile("slow.png", [tEXt("prompt", PROMPT)]);
const aborted = new AbortController();
const reading = m.localFileAdapter.read(await handleFor(file), { signal: aborted.signal });
const settled = (promise) => Promise.race([promise.then(() => "resolved", (error) => error.name), new Promise((resolve) => setTimeout(() => resolve("pending"), 200))]);
await new Promise((resolve) => setTimeout(resolve, 20));
aborted.abort();
assert.equal(await settled(reading), "AbortError");
// Through the controller: clearing the selection ends its read; nothing is published for it.
const controller = createInspectionController({ api: null, adapter: m.localFileAdapter, extractFacts: extractGenerationFacts });
const states = [];
controller.subscribe((state) => states.push(state));
const selected = controller.select(await handleFor(file));
await new Promise((resolve) => setTimeout(resolve, 20));
controller.clear();
assert.equal(await settled(selected), "resolved");
assert.ok(states.every((state) => state.record === null && state.error === false), "no record or error for a cleared selection");
controller.dispose();
} finally {
tableGate = null;
release();
}
// The shared request still lands for the next selection.
const { wire } = await recordFor(pngFile("next.png", [tEXt("prompt", PROMPT)]));
assert.equal(wire.resolution.state, "resolved");
});
test("a PNG File becomes the server-shaped wire with local provenance", async () => {
const file = pngFile("Fox Render.PNG", [tEXt("prompt", PROMPT), tEXt("workflow", WORKFLOW)]);
const { wire, record, handle } = await recordFor(file);
@@ -345,3 +379,89 @@ test("the parity corpus PNGs read into renderable records", async () => {
assert.ok(["parsed", "limited", "malformed"].includes(wire.metadata.prompt_graph.status));
}
});
// M4: resolution runs on the page's main thread, so a read yields between its bounded steps (the busy state paints
// and input is handled) and no step may exceed a budget on the largest record the readers keep (2 MiB of roles).
const ROLE_BYTES = 2 * 1024 * 1024;
// Generous for a shared CI runner: a desktop resolves these in 130-455 ms.
const STEP_BUDGET_MS = 2500;
const ENCODE = (text) => ({ class_type: "CLIPTextEncode", inputs: { text, clip: ["4", 1] } });
const KSAMPLER = (positive, negative, latent) => ({ class_type: "KSampler", inputs: { seed: 1, steps: 20, cfg: 8.0, sampler_name: "euler", scheduler: "normal",
denoise: 1.0, model: ["4", 0], positive, negative, latent_image: latent } });
const LOADERS = { "4": { class_type: "CheckpointLoaderSimple", inputs: { ckpt_name: "m.safetensors" } }, "5": { class_type: "EmptyLatentImage", inputs: { width: 64, height: 64, batch_size: 1 } } };
/** The largest graph of this shape whose JSON fits the role budget. */
function maximal(build, high) {
let low = 1;
while (low < high) {
const middle = (low + high + 1) >> 1;
if (Buffer.byteLength(JSON.stringify(build(middle)), "utf8") <= ROLE_BYTES) low = middle;
else high = middle - 1;
}
return JSON.stringify(build(low));
}
const MAXIMAL_GRAPHS = {
// Many candidates: every sampler is a final stage with its own pair.
independent: maximal((count) => {
const graph = { ...LOADERS };
for (let index = 0; index < count; index++) {
graph[`s${index}`] = KSAMPLER([`p${index}`, 0], [`n${index}`, 0], ["5", 0]);
graph[`p${index}`] = ENCODE(`fox ${index}`);
graph[`n${index}`] = ENCODE(`blur ${index}`);
}
return graph;
}, 6666),
// One long stage chain through the latent: the deepest stage walk.
chain: maximal((count) => {
const graph = { ...LOADERS, "6": ENCODE("fox"), "7": ENCODE("blur") };
for (let index = 0; index < count; index++) graph[`s${index}`] = KSAMPLER(["6", 0], ["7", 0], index ? [`s${index - 1}`, 0] : ["5", 0]);
return graph;
}, 19996),
};
test("resolution of a maximal 2 MiB graph stays within the step budget", async () => {
const { resolveLocalEvidence } = await import("../../web/js/workspace/local-resolver.mjs");
const semantics = await (await import("../../web/js/workspace/local-semantics.mjs")).loadInspectionSemantics();
for (const [name, prompt] of Object.entries(MAXIMAL_GRAPHS)) {
assert.ok(Buffer.byteLength(prompt, "utf8") > ROLE_BYTES - 1024, `${name} fills the role budget`);
const started = performance.now();
const evidence = resolveLocalEvidence({ prompt }, semantics);
const elapsed = performance.now() - started;
assert.ok(elapsed < STEP_BUDGET_MS, `${name}: ${Math.round(elapsed)} ms`);
assert.notEqual(evidence.resolution.reason_code, "malformed", name);
}
});
test("a read yields between bounded steps and answers byte for byte as one synchronous build", async () => {
const semantics = await (await import("../../web/js/workspace/local-semantics.mjs")).loadInspectionSemantics();
const file = pngFile("maximal.png", [tEXt("prompt", MAXIMAL_GRAPHS.independent)]);
const handle = await handleFor(file);
const marks = [];
const yieldControl = async (signal) => {
signal?.throwIfAborted();
marks.push(performance.now());
await new Promise((resolve) => setTimeout(resolve, 0));
};
const started = performance.now();
const wire = await m.localFileAdapter.read(handle, { signal: new AbortController().signal, yieldControl });
marks.push(performance.now());
// Resolution and the projection each run in a turn of their own after the table is in.
assert.ok(marks.length >= 3, `yields: ${marks.length - 1}`);
const gaps = marks.map((mark, index) => mark - (index ? marks[index - 1] : started));
assert.ok(Math.max(...gaps) < STEP_BUDGET_MS, `longest step ${Math.round(Math.max(...gaps))} ms`);
const { result } = await m.readLocalFile(file, { classification: handle.classification });
const direct = await m.fitLocalPayload(m.buildLocalPayload({ read: result, file: { name: handle.filename, size: file.size, lastModified: file.lastModified },
classification: handle.classification, semantics }));
assert.equal(JSON.stringify(wire), JSON.stringify(direct));
assert.equal(wire.resolution.state, "ambiguous");
// An abort at any yield ends the read there.
for (let stop = 0; stop < marks.length - 1; stop++) {
const controller = new AbortController();
let seen = 0;
const stopping = async (signal) => {
if (seen++ === stop) controller.abort();
signal.throwIfAborted();
await new Promise((resolve) => setTimeout(resolve, 0));
};
await assert.rejects(m.localFileAdapter.read(handle, { signal: controller.signal, yieldControl: stopping }), { name: "AbortError" }, `yield ${stop}`);
}
});
+2 -2
View File
@@ -193,8 +193,8 @@ test("unclassified input creates no object URL", async () => {
// declares a supported type it opens the inspector's unreadable state, with no preview and no URL (ledger ruling).
const gone = pngFile("gone.png", "fox", { gate: () => Promise.reject(new DOMException("The file was removed", "NotReadableError")) });
assert.equal(value.select([gone]), "accepted");
await settle();
const unreadable = value.controller.state;
// The read yields between its steps (local-inspection.mjs): wait for the answer, not a fixed number of turns.
const unreadable = await recorded(value.controller);
assert.ok(unreadable.record, "the unreadable record is published");
assert.deepEqual([value.state.phase, value.state.message, value.state.preview], ["ready", m.LOCAL_SESSION_MESSAGES.unreadable, null]);
assert.equal(m.LOCAL_SESSION_MESSAGES.unreadable, "Embedded metadata could not be read.");
+4 -2
View File
@@ -205,7 +205,7 @@ export function mountFileInspector(container, {api, sender, parentScope, variant
scope.listen(container, 'keydown', onInteraction);
scope.listen(container, 'click', onInteraction);
scope.listen(container, 'wheel', onInteraction, {passive:true});
scope.cleanup(() => {contentScope?.dispose(); container.replaceChildren(); container.classList.remove('file-inspector');});
scope.cleanup(() => {contentScope?.dispose(); container.replaceChildren(); container.classList.remove('file-inspector'); container.removeAttribute('aria-busy');});
const full = (variant ?? (inline ? 'panel' : 'full')) === 'full';
function statusView(state, parent) {
@@ -224,6 +224,8 @@ export function mountFileInspector(container, {api, sender, parentScope, variant
if (scope.signal.aborted) return;
contentScope?.dispose(); contentScope = childScope(scope);
container.replaceChildren();
// A local read resolves on this thread in bounded steps; the busy state stays until its answer lands.
if (state.loading) container.setAttribute('aria-busy', 'true'); else container.removeAttribute('aria-busy');
render(state);
}
/**
@@ -290,7 +292,7 @@ export function mountFileInspector(container, {api, sender, parentScope, variant
* copy does: the shared toast, or the select-to-copy dialog where the clipboard is unavailable or refused.
*/
function summaryButton(record) {
const button = createButton('Copy settings summary'), text = settingsSummary(record);
const button = createButton('Copy settings summary', 'settings-summary'), text = settingsSummary(record);
contentScope.listen(button, 'click', async () => {
try {
if (!navigator.clipboard) throw new Error('Clipboard requires a secure context');
+38 -6
View File
@@ -18,6 +18,24 @@ const MAX_FILENAME_CHARS = 255;
const MEDIA_TYPES = new Set(['image', 'video']);
const object = value => value !== null && typeof value === 'object' && !Array.isArray(value);
const abortError = () => new DOMException('The local selection ended', 'AbortError');
/** `promise`, or an AbortError as soon as `signal` aborts (the work behind `promise` is shared and keeps going). */
function untilAborted(promise, signal) {
if (!signal) return promise;
if (signal.aborted) return Promise.reject(abortError());
return new Promise((resolve, reject) => {
const onAbort = () => reject(abortError());
signal.addEventListener('abort', onAbort, { once: true });
promise.then(value => { signal.removeEventListener('abort', onAbort); resolve(value); },
error => { signal.removeEventListener('abort', onAbort); reject(error); });
});
}
/** One turn of the event loop between bounded steps, so the busy state paints and input is handled. */
function nextTurn(signal) {
return untilAborted(new Promise(resolve => setTimeout(resolve, 0)), signal);
}
const pngReader = () => globalThis.PngMetadata || globalThis.window?.PngMetadata;
/** A display name with no path separators or control characters; never empty. */
@@ -87,16 +105,21 @@ export function toInspectionRead(classification, result) {
const dimension = value => Number.isSafeInteger(value) && value > 0 ? value : null;
/** The roles for one local read (`{prompts, resolution}`): the bounded parse and resolution step of buildLocalPayload. */
export function localEvidence(read, classification, semantics = null) {
const inspection = toInspectionRead(classificationOf(classification), read);
return inspection.read === null ? unreadableEvidence() : resolveLocalEvidence(inspection.read.records, semantics);
}
/**
* The `inspection_version: 1` wire for one local read, before the response budget. Its roles are the server
* inspector's for the same records (local-resolver.mjs with `semantics`, the compiled node table; null answers
* "unavailable" as the server does), so the legacy fill in projectInspectionPayload applies exactly where it does
* for Images.
*/
export function buildLocalPayload({ read, file, classification, semantics = null }) {
export function buildLocalPayload({ read, file, classification, semantics = null, evidence = localEvidence(read, classification, semantics) }) {
const classified = classificationOf(classification);
const inspection = toInspectionRead(classified, read);
const evidence = inspection.read === null ? unreadableEvidence() : resolveLocalEvidence(inspection.read.records, semantics);
const filename = sanitizeLocalFilename(file?.name);
const size = Number.isSafeInteger(file?.size) && file.size >= 0 ? file.size : null;
const modified = typeof file?.lastModified === 'number' && Number.isFinite(file.lastModified) && file.lastModified >= 0 ? file.lastModified / 1000 : null;
@@ -171,7 +194,12 @@ export const localFileAdapter = Object.freeze({
scope(handle) { owned(handle); return LOCAL_SCOPE; },
// An inert sentinel: the controller's same-key check works and no URL is ever built.
inspectionPath(handle) { return `local-file:${owned(handle).key}`; },
async read(handle, { signal } = {}) {
/**
* Reads, resolves and projects one selection on the page's thread in bounded steps (each bound by the 2 MiB role
* and 4 MiB workflow budgets and the resolver's work limits) with a turn of the event loop between them, so the
* inspector's busy state paints. Every wait ends with the selection's `signal`; `yieldControl` is for tests.
*/
async read(handle, { signal, yieldControl = nextTurn } = {}) {
const secret = owned(handle);
let result = null;
try {
@@ -181,10 +209,14 @@ export const localFileAdapter = Object.freeze({
// MetadataReadError, an unreadable record. Abort and programming errors still reject.
if (!(error instanceof DOMException) || error.name === 'AbortError' || signal?.aborted) throw error;
}
const semantics = await loadInspectionSemantics();
signal?.throwIfAborted();
const semantics = await untilAborted(loadInspectionSemantics(), signal);
await yieldControl(signal);
const evidence = localEvidence(result, secret.classification, semantics);
await yieldControl(signal);
const file = { name: secret.filename, size: secret.file.size, lastModified: secret.file.lastModified };
return fitLocalPayload(buildLocalPayload({ read: result, file, classification: secret.classification, semantics }), MAX_RESPONSE_BYTES, { signal });
const payload = buildLocalPayload({ read: result, file, classification: secret.classification, semantics, evidence });
await yieldControl(signal);
return fitLocalPayload(payload, MAX_RESPONSE_BYTES, { signal });
},
toRecord(result, handle, extractFacts) {
const secret = owned(handle);
+1 -1
View File
@@ -213,7 +213,7 @@ main:focus-visible { outline: 2px solid var(--ca-focus); outline-offset: 4px; }
where the toolkit's `nowrap` would push them past its edge. */
.ca-tools #viewer-details > button { white-space: normal; /* D20 */ }
/* LI-4: "Copy settings summary" after the facts wraps the same way in the narrow rail. */
.ca-tools #viewer-details > div > button { white-space: normal; /* D117 */ }
.ca-tools #viewer-details .settings-summary { white-space: normal; /* D117 */ }
/* The Images page narrows to one column inside a narrow container (the Library-embedded gallery). */
.images-main { container: images-page / inline-size; }
@container images-page (inline-size < 600px) {