fix: preserve subgraph instance context on cached node reuse, deduplicate workflow analysis requests, and configure context menu for dynamic nodes
This commit is contained in:
+15
-13
@@ -323,14 +323,18 @@ def analyze_workflow_models(
|
||||
and cached_node.get("fingerprint") == node_fingerprint
|
||||
)
|
||||
if reused:
|
||||
model_refs = cached_node.get("refs", [])
|
||||
base_model_refs = cached_node.get("refs", [])
|
||||
reused_nodes += 1
|
||||
else:
|
||||
model_refs = references.get_node_model_info(
|
||||
base_model_refs = references.get_node_model_info(
|
||||
node,
|
||||
available_models=available_models,
|
||||
)
|
||||
analyzed_nodes += 1
|
||||
# Keep cached node references as raw extraction results. Subgraph
|
||||
# instance context is workflow-level state and must be reapplied
|
||||
# when a cached node is reused.
|
||||
model_refs = [dict(ref) for ref in base_model_refs]
|
||||
node_type = node.get("type", "")
|
||||
|
||||
# Check if node type is a subgraph UUID
|
||||
@@ -352,20 +356,18 @@ def analyze_workflow_models(
|
||||
promoted_instance_slots.add(
|
||||
(str(node.get("id")), ref.get("widget_index"))
|
||||
)
|
||||
if not reused:
|
||||
subgraph_utils._apply_instance_promoted_widget_context(
|
||||
ref, context, available_models
|
||||
)
|
||||
if not reused:
|
||||
for ref in model_refs:
|
||||
ref["subgraph_id"] = subgraph_id
|
||||
ref["subgraph_name"] = subgraph_name
|
||||
ref["subgraph_path"] = None
|
||||
ref["is_top_level"] = True
|
||||
subgraph_utils._apply_instance_promoted_widget_context(
|
||||
ref, context, available_models
|
||||
)
|
||||
for ref in model_refs:
|
||||
ref["subgraph_id"] = subgraph_id
|
||||
ref["subgraph_name"] = subgraph_name
|
||||
ref["subgraph_path"] = None
|
||||
ref["is_top_level"] = True
|
||||
if node_cache_out is not None:
|
||||
node_cache_out[node_cache_key] = {
|
||||
"fingerprint": node_fingerprint,
|
||||
"refs": model_refs,
|
||||
"refs": base_model_refs,
|
||||
}
|
||||
report_node_progress(node, reused=reused)
|
||||
all_model_refs.extend(model_refs)
|
||||
|
||||
@@ -344,6 +344,14 @@ def _apply_instance_promoted_widget_context(
|
||||
ref["promoted_inner_widget_index"] = context.get("widget_index")
|
||||
ref["promoted_inner_widget_name"] = context.get("widget_name", "")
|
||||
|
||||
promoted_value = context.get("promoted_value")
|
||||
nested_key = ref.get("nested_key")
|
||||
if nested_key and isinstance(promoted_value, dict):
|
||||
promoted_value = promoted_value.get(nested_key)
|
||||
if _has_promoted_value(promoted_value):
|
||||
ref["original_path"] = str(promoted_value).strip()
|
||||
ref["is_urn"] = bool(references.URN_REGEX.match(ref["original_path"]))
|
||||
|
||||
category = context.get("category")
|
||||
original_path = ref.get("original_path", "")
|
||||
if not category or not original_path:
|
||||
|
||||
@@ -1912,6 +1912,10 @@ test('background Missing Models refresh keeps the current view until new data is
|
||||
cachedAnalysisData: { missing_models: [], resolved_models: [] },
|
||||
syncWorkflowScopedQueue() {},
|
||||
getMissingWorkflowSignature: () => 'new-signature',
|
||||
getWorkflowAnalysisRequest: () => ({
|
||||
analysisId: 'shared-analysis',
|
||||
promise: fetchPromise,
|
||||
}),
|
||||
workflowHasNodes: () => true,
|
||||
renderAnalysisProgress() {
|
||||
progressRenderCount += 1;
|
||||
@@ -1958,6 +1962,42 @@ test('background Missing Models refresh keeps the current view until new data is
|
||||
assert.deepEqual(displayOptions, { preserveBrowser: true });
|
||||
});
|
||||
|
||||
test('concurrent workflow analysis consumers share one in-flight request', async () => {
|
||||
const getWorkflowAnalysisRequest = eval(
|
||||
`(${extractMethod(workflowStateMethodsSource, 'getWorkflowAnalysisRequest')})`
|
||||
);
|
||||
const workflow = { nodes: [{ id: 1, type: 'CheckpointLoaderSimple', widgets_values: ['model.safetensors'] }] };
|
||||
let resolveFirst;
|
||||
const pending = new Promise(resolve => {
|
||||
resolveFirst = resolve;
|
||||
});
|
||||
const calls = [];
|
||||
const responses = [pending, Promise.resolve({})];
|
||||
const dialog = {
|
||||
getMissingWorkflowSignature: () => 'model-signature',
|
||||
getActiveWorkflowRouteKey: () => 'workflow-a',
|
||||
fetchJson: (...args) => {
|
||||
calls.push(args);
|
||||
return responses.shift();
|
||||
},
|
||||
};
|
||||
|
||||
const first = getWorkflowAnalysisRequest.call(dialog, workflow);
|
||||
const second = getWorkflowAnalysisRequest.call(dialog, workflow);
|
||||
|
||||
assert.equal(first, second);
|
||||
assert.equal(calls.length, 1);
|
||||
assert.match(calls[0][0], /\/model_resolver\/analyze$/);
|
||||
|
||||
resolveFirst({ missing_models: [] });
|
||||
await first.promise;
|
||||
await Promise.resolve();
|
||||
|
||||
const next = getWorkflowAnalysisRequest.call(dialog, workflow);
|
||||
assert.notEqual(next, first);
|
||||
assert.equal(calls.length, 2);
|
||||
});
|
||||
|
||||
test('content-preserving Missing Models refresh patches the browser instead of clearing it', () => {
|
||||
const patchMissingModelsBrowserElement = extractMethod(
|
||||
missingBrowserMethodsSource,
|
||||
@@ -2964,6 +3004,36 @@ test('workflow hash refresh ignores node movement and tracks model dependency ch
|
||||
assert.notEqual(resolver.workflowHashMetadataPendingSignature, originalSignature);
|
||||
});
|
||||
|
||||
test('workflow route stays rooted while navigating into a subgraph', () => {
|
||||
const getActiveWorkflowRouteKey = eval(
|
||||
`(${extractMethod(workflowStateMethodsSource, 'getActiveWorkflowRouteKey')})`
|
||||
);
|
||||
const previousApp = globalThis.app;
|
||||
const previousWindow = globalThis.window;
|
||||
|
||||
globalThis.app = {
|
||||
graph: {
|
||||
id: 'root-workflow-id',
|
||||
rootGraph: { id: 'root-workflow-id' },
|
||||
},
|
||||
canvas: {
|
||||
graph: { id: 'nested-subgraph-id' },
|
||||
},
|
||||
};
|
||||
globalThis.window = {
|
||||
location: { hash: '#nested-subgraph-id' },
|
||||
};
|
||||
|
||||
try {
|
||||
assert.equal(getActiveWorkflowRouteKey.call({}), '#root-workflow-id');
|
||||
} finally {
|
||||
if (previousApp === undefined) delete globalThis.app;
|
||||
else globalThis.app = previousApp;
|
||||
if (previousWindow === undefined) delete globalThis.window;
|
||||
else globalThis.window = previousWindow;
|
||||
}
|
||||
});
|
||||
|
||||
test('download percent keeps native Xet progress below one percent visible', () => {
|
||||
const formatDownloadPercent = eval(`(${extractMethod(renderFormatMethodsSource, 'formatDownloadPercent')})`);
|
||||
|
||||
|
||||
@@ -34,4 +34,10 @@ test('frontend entrypoint registers the ComfyUI extension contract', () => {
|
||||
`expected ComfyUI lifecycle hook: ${hook}`
|
||||
);
|
||||
}
|
||||
|
||||
assert.equal(
|
||||
(resolverSource.match(/modelResolver\.configureNodeContextMenu\(node\?\.constructor\);/g) || []).length,
|
||||
2,
|
||||
'dynamically created and loaded node types must receive the widget-change hook'
|
||||
);
|
||||
});
|
||||
|
||||
@@ -261,6 +261,68 @@ def test_subgraph_input_reference_targets_instance_for_update_and_location():
|
||||
]
|
||||
|
||||
|
||||
def test_reused_subgraph_instance_refreshes_promoted_model_context():
|
||||
workflow = {
|
||||
"nodes": [
|
||||
{
|
||||
"id": 1,
|
||||
"type": "subgraph-1",
|
||||
"inputs": [
|
||||
{
|
||||
"name": "ckpt_name",
|
||||
"type": "COMBO",
|
||||
"widget": {"name": "ckpt_name"},
|
||||
"link": None,
|
||||
}
|
||||
],
|
||||
"properties": {"proxyWidgets": [["10", "ckpt_name"]]},
|
||||
"widgets_values": ["old.safetensors"],
|
||||
}
|
||||
],
|
||||
"definitions": {
|
||||
"subgraphs": [
|
||||
{
|
||||
"id": "subgraph-1",
|
||||
"name": "Promoted loader",
|
||||
"nodes": [_loader_node(value="old.safetensors")],
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
previous_node_cache = {}
|
||||
analysis.analyze_workflow_models(
|
||||
workflow,
|
||||
available_models=[],
|
||||
node_cache_out=previous_node_cache,
|
||||
)
|
||||
|
||||
updated_workflow = {
|
||||
**workflow,
|
||||
"nodes": [
|
||||
{
|
||||
**workflow["nodes"][0],
|
||||
"widgets_values": ["new.safetensors"],
|
||||
}
|
||||
],
|
||||
}
|
||||
# Simulate a serialized proxy widget change whose node cache entry was
|
||||
# retained while the promoted context changed.
|
||||
previous_node_cache[("top", "", "1")]["fingerprint"] = (
|
||||
analysis._get_workflow_node_fingerprint(updated_workflow["nodes"][0])
|
||||
)
|
||||
|
||||
refs = analysis.analyze_workflow_models(
|
||||
updated_workflow,
|
||||
available_models=[],
|
||||
previous_node_cache=previous_node_cache,
|
||||
)
|
||||
|
||||
assert len(refs) == 1
|
||||
assert refs[0]["node_id"] == 1
|
||||
assert refs[0]["original_path"] == "new.safetensors"
|
||||
assert refs[0]["promoted_inner_node_id"] == 10
|
||||
|
||||
|
||||
def test_reference_matching_handles_placeholders_paths_and_scanner_records(tmp_path):
|
||||
model_path = tmp_path / "folder" / "model.safetensors"
|
||||
model_path.parent.mkdir()
|
||||
|
||||
@@ -34,11 +34,13 @@ app.registerExtension({
|
||||
modelResolver.configureNodeContextMenu(nodeType);
|
||||
},
|
||||
nodeCreated(node) {
|
||||
modelResolver.configureNodeContextMenu(node?.constructor);
|
||||
modelResolver.configureWorkflowDependencyMarkerNode(node);
|
||||
modelResolver.configureCustomNodeModelAdapter(node);
|
||||
modelResolver.scheduleNodeContextMenuAnalysis();
|
||||
},
|
||||
loadedGraphNode(node) {
|
||||
modelResolver.configureNodeContextMenu(node?.constructor);
|
||||
modelResolver.configureWorkflowDependencyMarkerNode(node);
|
||||
modelResolver.configureCustomNodeModelAdapter(node);
|
||||
modelResolver.scheduleNodeContextMenuAnalysis();
|
||||
|
||||
@@ -360,11 +360,12 @@ export class ModelResolver {
|
||||
if (!force && cachedData) return cachedData;
|
||||
if (this.nodeContextAnalysisPromise) return this.nodeContextAnalysisPromise;
|
||||
|
||||
const analysisPromise = this.dialog.fetchJson('/model_resolver/analyze', {
|
||||
method: 'POST',
|
||||
body: JSON.stringify({ workflow }),
|
||||
const analysisRequest = this.dialog.getWorkflowAnalysisRequest?.(workflow, {
|
||||
silent: true,
|
||||
}, 'Analyze workflow')
|
||||
});
|
||||
if (!analysisRequest?.promise) return null;
|
||||
|
||||
const analysisPromise = analysisRequest.promise
|
||||
.then((data) => {
|
||||
const currentState = this.getNodeContextWorkflowState();
|
||||
if (currentState.signature === signature) {
|
||||
|
||||
@@ -120,6 +120,7 @@ export class ResolverManagerDialog extends ComfyDialog {
|
||||
this._sidebarSplitterMouseDownHandler = null;
|
||||
this._sidebarSplitterDragState = null;
|
||||
this._analysisProgressToken = null;
|
||||
this._workflowAnalysisRequest = null;
|
||||
this._workflowDataLoadToken = null;
|
||||
this._loadedModelsLoadToken = null;
|
||||
this._loadedModelsProgressToken = null;
|
||||
|
||||
@@ -201,8 +201,12 @@ export const lifecycleGraphMethods = {
|
||||
return data;
|
||||
}
|
||||
|
||||
const analysisId = `an-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
|
||||
this._analysisProgressToken = analysisId;
|
||||
const analysisRequest = this.getWorkflowAnalysisRequest(workflow, {
|
||||
forceRescan: Boolean(forceRescan),
|
||||
});
|
||||
if (!analysisRequest) return null;
|
||||
|
||||
this._analysisProgressToken = analysisRequest.analysisId;
|
||||
if (!preserveContent && shouldRenderMissingModels()) {
|
||||
this.contentElement.innerHTML = this.renderAnalysisProgress({
|
||||
status: 'starting',
|
||||
@@ -215,15 +219,15 @@ export const lifecycleGraphMethods = {
|
||||
// Call analyze endpoint
|
||||
const progressPromise = preserveContent
|
||||
? Promise.resolve()
|
||||
: this.pollAnalysisProgress(analysisId, analysisId);
|
||||
: this.pollAnalysisProgress(
|
||||
analysisRequest.analysisId,
|
||||
analysisRequest.analysisId
|
||||
);
|
||||
let data;
|
||||
try {
|
||||
data = await this.fetchJson('/model_resolver/analyze', {
|
||||
method: 'POST',
|
||||
body: JSON.stringify({ workflow, analysis_id: analysisId, force_rescan: Boolean(forceRescan) })
|
||||
}, 'Analyze workflow');
|
||||
data = await analysisRequest.promise;
|
||||
} finally {
|
||||
if (this._analysisProgressToken === analysisId) {
|
||||
if (this._analysisProgressToken === analysisRequest.analysisId) {
|
||||
this._analysisProgressToken = null;
|
||||
}
|
||||
await progressPromise;
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { app } from "../../../../../scripts/app.js";
|
||||
import { createModuleLogger } from "../../log_system/log_funcs.js";
|
||||
|
||||
const log = createModuleLogger('workflow_state_methods');
|
||||
@@ -17,6 +18,53 @@ export const workflowStateMethods = {
|
||||
return this.getWorkflowSignature(workflow, { includeStrength: false });
|
||||
},
|
||||
|
||||
getWorkflowAnalysisRequest(workflow, { forceRescan = false, silent = false } = {}) {
|
||||
if (!workflow) return null;
|
||||
|
||||
const signature = this.getMissingWorkflowSignature(workflow);
|
||||
const route = this.getActiveWorkflowRouteKey?.() || '';
|
||||
const existing = this._workflowAnalysisRequest;
|
||||
if (
|
||||
!forceRescan
|
||||
&& existing?.signature === signature
|
||||
&& existing?.route === route
|
||||
) {
|
||||
return existing;
|
||||
}
|
||||
|
||||
const analysisId = `an-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
|
||||
const requestOptions = {
|
||||
method: 'POST',
|
||||
body: JSON.stringify({
|
||||
workflow,
|
||||
analysis_id: analysisId,
|
||||
force_rescan: Boolean(forceRescan),
|
||||
}),
|
||||
};
|
||||
if (silent) requestOptions.silent = true;
|
||||
|
||||
const request = {
|
||||
analysisId,
|
||||
promise: this.fetchJson('/model_resolver/analyze', requestOptions, 'Analyze workflow'),
|
||||
route,
|
||||
signature,
|
||||
};
|
||||
this._workflowAnalysisRequest = request;
|
||||
request.promise.then(
|
||||
() => {
|
||||
if (this._workflowAnalysisRequest === request) {
|
||||
this._workflowAnalysisRequest = null;
|
||||
}
|
||||
},
|
||||
() => {
|
||||
if (this._workflowAnalysisRequest === request) {
|
||||
this._workflowAnalysisRequest = null;
|
||||
}
|
||||
}
|
||||
);
|
||||
return request;
|
||||
},
|
||||
|
||||
isWorkflowStrengthWidgetName(name = '') {
|
||||
const normalized = String(name || '').trim().replace(/[-\s]/g, '_').toLowerCase();
|
||||
return normalized === 'strength_model'
|
||||
@@ -357,6 +405,10 @@ export const workflowStateMethods = {
|
||||
|
||||
getActiveWorkflowRouteKey() {
|
||||
try {
|
||||
const rootGraph = app?.rootGraph || app?.graph?.rootGraph || app?.graph;
|
||||
const rootGraphId = rootGraph?.id || rootGraph?._id || rootGraph?.config?.id;
|
||||
if (rootGraphId) return `#${rootGraphId}`;
|
||||
|
||||
return window.location?.hash || '';
|
||||
} catch (_error) {
|
||||
return '';
|
||||
|
||||
Reference in New Issue
Block a user