diff --git a/docs/nodes.md b/docs/nodes.md index 25fd875..97d6be2 100644 --- a/docs/nodes.md +++ b/docs/nodes.md @@ -18,7 +18,7 @@ default `knowledge_base/` works regardless of ComfyUI's working directory. | Node | Inputs | Outputs | |---|---|---| -| **Scene Analyzer** | `natural_language` (STRING), `model_name` (STRING), `temperature` (FLOAT), `timeout` (INT), `system_prompt` (STRING, optional) | `scene` (SCENE), `warnings` (STRING), `errors` (STRING) | +| **Scene Analyzer** | `natural_language` (STRING), `config` (COMPILER_CONFIG, optional — supplies model/temperature/retries/timeout), `system_prompt` (STRING, optional override) | `scene` (SCENE), `warnings` (STRING), `errors` (STRING) | | **Scene Validator** | `scene` (SCENE), `config` (COMPILER_CONFIG, optional) | `scene` (SCENE), `warnings` (STRING), `errors` (STRING) | | **Resolver** | `scene` (SCENE), `knowledge_base` (KNOWLEDGE_BASE), `config` (COMPILER_CONFIG, optional) | `resolved_tags` (RESOLVED_TAGS), `warnings` (STRING), `errors` (STRING) | | **Category Splitter** | `resolved_tags` (RESOLVED_TAGS) | `category_map` (CATEGORY_MAP), `warnings` (STRING), `errors` (STRING) | @@ -32,12 +32,13 @@ prompt strings are produced by the **Prompt Builder**. | Node | Inputs | Outputs | |---|---|---| | **Configuration** | one input per configuration option (analyzer, resolver, validator, prompt builder, debug) | `config` (COMPILER_CONFIG), `errors` (STRING) | -| **Knowledge Base Loader** | `path` (STRING), `reload` (INT, optional) | `knowledge_base` (KNOWLEDGE_BASE), `warnings` (STRING), `errors` (STRING) | +| **Knowledge Base Loader** | `path` (STRING), `reload` (INT, optional), `version` (STRING, optional), `config` (COMPILER_CONFIG, optional — its KB path/version win) | `knowledge_base` (KNOWLEDGE_BASE), `warnings` (STRING), `errors` (STRING) | | **Debug Viewer** | `scene` (SCENE, optional), `resolved_tags` (RESOLVED_TAGS, optional), `category_map` (CATEGORY_MAP, optional), `warnings` (STRING, optional), `errors` (STRING, optional) | `report` (STRING) | - The **Configuration** node's `config` output feeds the optional `config` input of - the Validator, Resolver, and Prompt Builder, so behaviour changes without - rewiring. + the Scene Analyzer, Validator, Resolver, and Knowledge Base Loader, so behaviour + (including the analyzer model and the Knowledge Base path/version) changes in one + place without rewiring. - The **Knowledge Base Loader** reloads only when `reload` changes (no automatic file watching). - The **Debug Viewer** is read-only; connect any intermediate state to inspect it. diff --git a/nodes/configuration_node.py b/nodes/configuration_node.py index 9cbd1a2..bc8840b 100644 --- a/nodes/configuration_node.py +++ b/nodes/configuration_node.py @@ -28,37 +28,165 @@ class ConfigurationNode: def INPUT_TYPES(cls) -> dict[str, Any]: return { "required": { - "analyzer_model": ("STRING", {"default": "llama3"}), + "analyzer_model": ( + "STRING", + { + "default": "llama3", + "tooltip": "Ollama model the Scene Analyzer uses to read the description.", + }, + ), "analyzer_temperature": ( "FLOAT", - {"default": 0.0, "min": 0.0, "max": 2.0, "step": 0.1}, + { + "default": 0.0, + "min": 0.0, + "max": 2.0, + "step": 0.1, + "tooltip": "Analyzer sampling temperature. 0 = most repeatable.", + }, + ), + "analyzer_max_retries": ( + "INT", + { + "default": 3, + "min": 0, + "max": 10, + "tooltip": "How many times to re-ask when the model returns bad JSON.", + }, + ), + "analyzer_timeout": ( + "INT", + { + "default": 300, + "min": 1, + "max": 3600, + "tooltip": ( + "Seconds to wait for the model. 300 s is generous so a model that " + "cold-loads into VRAM on the first call does not time out." + ), + }, + ), + "knowledge_base": ( + "STRING", + { + "default": "knowledge_base/", + "tooltip": ( + "Knowledge Base directory. Relative paths resolve against the node " + "package. This is the authoritative path when this Configuration is " + "wired into the Knowledge Base Loader." + ), + }, + ), + "resolver_strict_mode": ( + "BOOLEAN", + {"default": True, "tooltip": "Report unknown concepts instead of guessing."}, + ), + "resolver_allow_aliases": ( + "BOOLEAN", + {"default": True, "tooltip": "Resolve aliases to their canonical entry."}, + ), + "resolver_expansion_enabled": ( + "BOOLEAN", + {"default": True, "tooltip": "Auto-add tags from an entry's expansion list."}, + ), + "resolver_max_expansion_depth": ( + "INT", + { + "default": 8, + "min": 1, + "max": 32, + "tooltip": "How deep expansion may recurse before stopping.", + }, + ), + "resolver_include_nsfw": ( + "BOOLEAN", + { + "default": False, + "tooltip": "Include explicit-rated Knowledge Base entries. Off = SFW only.", + }, + ), + "validator_allow_unknown_fields": ( + "BOOLEAN", + { + "default": False, + "tooltip": "Keep unrecognized Scene JSON fields instead of stripping them.", + }, + ), + "prompt_target": ( + "STRING", + { + "default": "easy_illustrious", + "tooltip": "Reserved prompt-format label. Currently informational.", + }, + ), + "prompt_separator": ( + "STRING", + { + "default": ",", + "tooltip": "String used to join the resolved tags into the final prompt.", + }, + ), + "prompt_remove_duplicate_tags": ( + "BOOLEAN", + { + "default": True, + "tooltip": "Drop duplicate tags after expansion (warns SC0007).", + }, + ), + "debug_enabled": ( + "BOOLEAN", + {"default": False, "tooltip": "Enable extra diagnostic logging."}, + ), + "debug_level": ( + _DEBUG_LEVELS, + {"default": "basic", "tooltip": "Verbosity of debug logging when enabled."}, ), - "analyzer_max_retries": ("INT", {"default": 3, "min": 0, "max": 10}), - "analyzer_timeout": ("INT", {"default": 300, "min": 1, "max": 3600}), - "knowledge_base": ("STRING", {"default": "knowledge_base/"}), - "resolver_strict_mode": ("BOOLEAN", {"default": True}), - "resolver_allow_aliases": ("BOOLEAN", {"default": True}), - "resolver_expansion_enabled": ("BOOLEAN", {"default": True}), - "resolver_max_expansion_depth": ("INT", {"default": 8, "min": 1, "max": 32}), - "resolver_include_nsfw": ("BOOLEAN", {"default": False}), - "validator_allow_unknown_fields": ("BOOLEAN", {"default": False}), - "prompt_target": ("STRING", {"default": "easy_illustrious"}), - "prompt_separator": ("STRING", {"default": ","}), - "prompt_remove_duplicate_tags": ("BOOLEAN", {"default": True}), - "debug_enabled": ("BOOLEAN", {"default": False}), - "debug_level": (_DEBUG_LEVELS, {"default": "basic"}), }, # Appended at the END so saved workflows keep their positional widget # values (ComfyUI stores widget values positionally). "optional": { - "analyzer_system_prompt": ("STRING", {"multiline": True, "default": ""}), - "resolver_knowledge_base_version": ("STRING", {"default": ""}), - "semantic_enabled": ("BOOLEAN", {"default": False}), + "analyzer_system_prompt": ( + "STRING", + { + "multiline": True, + "default": "", + "tooltip": "Advanced: override the analyzer system prompt.", + }, + ), + "resolver_knowledge_base_version": ( + "STRING", + { + "default": "", + "tooltip": "Pin a KB dataset version. Empty = unpinned.", + }, + ), + "semantic_enabled": ( + "BOOLEAN", + { + "default": False, + "tooltip": ( + "Opt-in nearest-neighbour fallback for concepts that miss " + "deterministic lookup. Off by default; deterministic lookup wins." + ), + }, + ), "semantic_min_similarity": ( "FLOAT", - {"default": 0.5, "min": 0.0, "max": 1.0, "step": 0.05}, + { + "default": 0.5, + "min": 0.0, + "max": 1.0, + "step": 0.05, + "tooltip": "Minimum similarity to accept a semantic-fallback match.", + }, + ), + "semantic_backend": ( + "STRING", + { + "default": "char_ngram", + "tooltip": "Embedding backend for the semantic fallback (offline).", + }, ), - "semantic_backend": ("STRING", {"default": "char_ngram"}), }, } diff --git a/nodes/debug_viewer_node.py b/nodes/debug_viewer_node.py index 8ca6690..953bbb8 100644 --- a/nodes/debug_viewer_node.py +++ b/nodes/debug_viewer_node.py @@ -26,9 +26,18 @@ class DebugViewerNode: def INPUT_TYPES(cls) -> dict[str, Any]: return { "optional": { - "scene": ("SCENE",), - "warnings": ("STRING", {"default": "", "forceInput": True}), - "errors": ("STRING", {"default": "", "forceInput": True}), + "scene": ( + "SCENE", + {"tooltip": "Scene JSON to inspect (Analyzer/Validator output)."}, + ), + "warnings": ( + "STRING", + {"default": "", "forceInput": True, "tooltip": "A stage's warnings output."}, + ), + "errors": ( + "STRING", + {"default": "", "forceInput": True, "tooltip": "A stage's errors output."}, + ), } } diff --git a/nodes/knowledge_base_loader_node.py b/nodes/knowledge_base_loader_node.py index e14f9f3..fe49c6d 100644 --- a/nodes/knowledge_base_loader_node.py +++ b/nodes/knowledge_base_loader_node.py @@ -14,6 +14,7 @@ from __future__ import annotations from pathlib import Path from typing import Any +from compiler.common.config import Config from compiler.common.knowledge_base import ( KnowledgeBaseError, KnowledgeBaseLoader, @@ -49,16 +50,65 @@ class KnowledgeBaseLoaderNode: @classmethod def INPUT_TYPES(cls) -> dict[str, Any]: return { - "required": {"path": ("STRING", {"default": "knowledge_base/"})}, + "required": { + "path": ( + "STRING", + { + "default": "knowledge_base/", + "tooltip": ( + "Knowledge Base directory. Relative paths resolve against the " + "node package, so the default works from any ComfyUI launch " + "directory. Overridden by a connected Configuration node." + ), + }, + ) + }, # New widgets are appended at the END so saved workflows keep their # positional widget values (ComfyUI stores widgets positionally). "optional": { - "reload": ("INT", {"default": 0, "min": 0}), - "version": ("STRING", {"default": ""}), + "reload": ( + "INT", + { + "default": 0, + "min": 0, + "tooltip": "Bump to force a fresh read of the Knowledge Base from disk.", + }, + ), + "version": ( + "STRING", + { + "default": "", + "tooltip": ( + "Optional Knowledge Base dataset version to pin (matched against " + "the dataset manifest). Empty means use whatever is on the path. " + "Overridden by a connected Configuration node." + ), + }, + ), + "config": ( + "COMPILER_CONFIG", + { + "tooltip": ( + "Optional Configuration node. When connected, its Knowledge Base " + "path and version take precedence over the widgets above." + ) + }, + ), }, } - def run(self, path: str, reload: int = 0, version: str = "") -> tuple[Any, str, str, str]: + def run( + self, + path: str, + reload: int = 0, + version: str = "", + config: Config | None = None, + ) -> tuple[Any, str, str, str]: + # The Configuration node is the single source of truth when wired: its + # Knowledge Base path/version win over this node's own widgets. + if config is not None: + path = config.resolver.knowledge_base + version = config.resolver.knowledge_base_version or "" resolved = _resolve_path(path) requested_version = version or None key = (str(resolved), version) diff --git a/nodes/resolver_node.py b/nodes/resolver_node.py index f967098..3c188af 100644 --- a/nodes/resolver_node.py +++ b/nodes/resolver_node.py @@ -30,10 +30,23 @@ class ResolverNode: def INPUT_TYPES(cls) -> dict[str, Any]: return { "required": { - "scene": ("SCENE",), - "knowledge_base": ("KNOWLEDGE_BASE",), + "scene": ("SCENE", {"tooltip": "Validated Scene JSON from the Scene Validator."}), + "knowledge_base": ( + "KNOWLEDGE_BASE", + {"tooltip": "Loaded Knowledge Base from the Knowledge Base Loader."}, + ), + }, + "optional": { + "config": ( + "COMPILER_CONFIG", + { + "tooltip": ( + "Optional Configuration node. Supplies resolver options, the prompt " + "separator, duplicate handling and the opt-in semantic fallback." + ) + }, + ) }, - "optional": {"config": ("COMPILER_CONFIG",)}, } def run( diff --git a/nodes/scene_analyzer_node.py b/nodes/scene_analyzer_node.py index 941d309..c81085b 100644 --- a/nodes/scene_analyzer_node.py +++ b/nodes/scene_analyzer_node.py @@ -3,6 +3,13 @@ A thin interface: it adapts ComfyUI inputs to the Analyzer module and surfaces the CompilerResult's data, warnings, and errors. It contains no compiler logic and never imports ComfyUI. + +Analyzer settings (model, temperature, retries, timeout, system prompt) live on +the **Configuration** node and reach this node through the optional ``config`` +input, so a setting like the model name is entered in exactly one place. Without a +Configuration node the built-in defaults apply (llama3, temperature 0), with a +generous 300 s timeout so a local Ollama model that cold-loads into VRAM on the +first call does not time out. """ from __future__ import annotations @@ -15,6 +22,10 @@ from compiler.common.config import Config from .adapters import format_messages +# Cold-load-friendly fallback timeout used only when no Configuration node is +# wired; a local model loading into VRAM on the first call can take minutes. +_DEFAULT_TIMEOUT = 300 + class SceneAnalyzerNode: """Runs the Scene Analyzer and emits Scene plus warnings/errors.""" @@ -28,33 +39,47 @@ class SceneAnalyzerNode: def INPUT_TYPES(cls) -> dict[str, Any]: return { "required": { - "natural_language": ("STRING", {"multiline": True, "default": ""}), - "model_name": ("STRING", {"default": "llama3"}), - "temperature": ("FLOAT", {"default": 0.0, "min": 0.0, "max": 2.0, "step": 0.1}), - "timeout": ("INT", {"default": 300, "min": 1, "max": 3600}), + "natural_language": ( + "STRING", + { + "multiline": True, + "default": "", + "tooltip": "The scene description to analyze, in plain language.", + }, + ), }, "optional": { - "system_prompt": ("STRING", {"multiline": True, "default": ""}), + "config": ( + "COMPILER_CONFIG", + { + "tooltip": ( + "Optional Configuration node. Supplies the analyzer model, " + "temperature, retries and timeout. If left unconnected, the " + "defaults apply (llama3, temperature 0, 300 s timeout)." + ) + }, + ), + "system_prompt": ( + "STRING", + { + "multiline": True, + "default": "", + "tooltip": ( + "Advanced: override the analyzer system prompt. Leave empty to " + "use the built-in prompt (or the one from the Configuration node)." + ), + }, + ), }, } def run( self, natural_language: str, - model_name: str, - temperature: float, - timeout: int, + config: Config | None = None, system_prompt: str = "", - ) -> tuple[Any, str, str]: - analyzer: dict[str, Any] = { - "model": model_name, - "temperature": temperature, - "timeout": timeout, - } - if system_prompt: - analyzer["system_prompt"] = system_prompt - - config = Config.from_json({"analyzer": analyzer}) + ) -> tuple[Any, str, str, str]: + config = self._resolve_config(config, system_prompt) backend = OllamaBackend.from_config(config) result = analyze(natural_language, backend, config) return ( @@ -66,3 +91,22 @@ class SceneAnalyzerNode: # re-serialization of the parsed Scene. result.metadata.get("raw_response", ""), ) + + @staticmethod + def _resolve_config(config: Config | None, system_prompt: str) -> Config: + """Pick the effective config, applying the optional system-prompt override. + + No Configuration node: build a default config with a cold-load-friendly + timeout. A non-empty ``system_prompt`` widget always wins over whatever the + Configuration node carries. + """ + if config is None: + analyzer: dict[str, Any] = {"timeout": _DEFAULT_TIMEOUT} + if system_prompt: + analyzer["system_prompt"] = system_prompt + return Config.from_json({"analyzer": analyzer}) + if system_prompt: + document = config.to_json() + document.setdefault("analyzer", {})["system_prompt"] = system_prompt + return Config.from_json(document) + return config diff --git a/nodes/scene_validator_node.py b/nodes/scene_validator_node.py index 86d3d61..b8c9775 100644 --- a/nodes/scene_validator_node.py +++ b/nodes/scene_validator_node.py @@ -26,8 +26,15 @@ class SceneValidatorNode: @classmethod def INPUT_TYPES(cls) -> dict[str, Any]: return { - "required": {"scene": ("SCENE",)}, - "optional": {"config": ("COMPILER_CONFIG",)}, + "required": { + "scene": ("SCENE", {"tooltip": "Raw Scene JSON from the Scene Analyzer."}), + }, + "optional": { + "config": ( + "COMPILER_CONFIG", + {"tooltip": "Optional Configuration node; supplies the unknown-field policy."}, + ) + }, } def run(self, scene: Any, config: Config | None = None) -> tuple[Any, str, str, str]: diff --git a/tests/test_knowledge_base_loader_node.py b/tests/test_knowledge_base_loader_node.py index fa20d7e..a88bd69 100644 --- a/tests/test_knowledge_base_loader_node.py +++ b/tests/test_knowledge_base_loader_node.py @@ -5,6 +5,7 @@ from __future__ import annotations import json import nodes +from compiler.common.config import Config from compiler.common.knowledge_base import KnowledgeBase from nodes.knowledge_base_loader_node import KnowledgeBaseLoaderNode @@ -60,6 +61,19 @@ def test_reload_input_is_accepted(tmp_path) -> None: assert isinstance(kb, KnowledgeBase) +def test_config_path_overrides_the_widget(tmp_path) -> None: + # A connected Configuration node is the single source of truth: its Knowledge + # Base path wins over the node's own ``path`` widget. + config_path = write_kb( + tmp_path, [{"id": "long_hair", "tags": ["long hair"], "category": "hair"}] + ) + config = Config.from_json({"resolver": {"knowledge_base": config_path}}) + kb, warnings, errors, *_ = KnowledgeBaseLoaderNode().run("does/not/exist", config=config) + assert isinstance(kb, KnowledgeBase) + assert kb.get("long_hair") is not None + assert (warnings, errors) == ("", "") + + def test_registered_for_comfyui() -> None: assert nodes.NODE_CLASS_MAPPINGS["SceneCompilerKnowledgeBaseLoader"] is KnowledgeBaseLoaderNode assert ( diff --git a/tests/test_scene_analyzer_node.py b/tests/test_scene_analyzer_node.py index 8b4df83..32c0bdb 100644 --- a/tests/test_scene_analyzer_node.py +++ b/tests/test_scene_analyzer_node.py @@ -4,6 +4,7 @@ from __future__ import annotations import nodes import nodes.scene_analyzer_node as node_module +from compiler.common.config import Config from compiler.common.message_codes import message from compiler.common.result import CompilerResult from nodes.scene_analyzer_node import SceneAnalyzerNode @@ -11,13 +12,10 @@ from nodes.scene_analyzer_node import SceneAnalyzerNode def test_input_types_structure() -> None: inputs = SceneAnalyzerNode.INPUT_TYPES() - assert set(inputs["required"]) == { - "natural_language", - "model_name", - "temperature", - "timeout", - } - assert "system_prompt" in inputs["optional"] + # Analyzer settings now live on the Configuration node; only the per-run text + # and the optional config/override remain. + assert set(inputs["required"]) == {"natural_language"} + assert set(inputs["optional"]) == {"config", "system_prompt"} def test_node_metadata() -> None: @@ -27,7 +25,7 @@ def test_node_metadata() -> None: assert SceneAnalyzerNode.CATEGORY == "Scene Compiler" -def test_run_delegates_to_analyzer_and_surfaces_messages(monkeypatch) -> None: +def test_run_uses_model_from_config(monkeypatch) -> None: captured = {} def fake_analyze(description, backend, config): @@ -41,11 +39,10 @@ def test_run_delegates_to_analyzer_and_surfaces_messages(monkeypatch) -> None: monkeypatch.setattr(node_module, "analyze", fake_analyze) + config = Config.from_json({"analyzer": {"model": "mistral"}}) scene, warnings, errors, *_ = SceneAnalyzerNode().run( natural_language="A girl.", - model_name="mistral", - temperature=0.0, - timeout=60, + config=config, ) assert scene == "SCENE_MODEL" assert captured["description"] == "A girl." @@ -54,13 +51,28 @@ def test_run_delegates_to_analyzer_and_surfaces_messages(monkeypatch) -> None: assert "SC0011: bad response" in errors +def test_defaults_apply_without_a_configuration_node(monkeypatch) -> None: + captured = {} + + def fake_analyze(description, backend, config): + captured["model"] = config.analyzer.model + captured["timeout"] = config.analyzer.timeout + return CompilerResult(data="S") + + monkeypatch.setattr(node_module, "analyze", fake_analyze) + SceneAnalyzerNode().run("A girl.") + # Falls back to the built-in model with a cold-load-friendly timeout. + assert captured["model"] == "llama3" + assert captured["timeout"] == 300 + + def test_no_messages_yield_empty_strings(monkeypatch) -> None: monkeypatch.setattr(node_module, "analyze", lambda d, b, c: CompilerResult(data="S")) - scene, warnings, errors, *_ = SceneAnalyzerNode().run("x", "llama3", 0.0, 60) + scene, warnings, errors, *_ = SceneAnalyzerNode().run("x") assert (warnings, errors) == ("", "") -def test_system_prompt_override_is_passed(monkeypatch) -> None: +def test_system_prompt_override_without_config(monkeypatch) -> None: captured = {} def fake_analyze(description, backend, config): @@ -68,10 +80,25 @@ def test_system_prompt_override_is_passed(monkeypatch) -> None: return CompilerResult(data="S") monkeypatch.setattr(node_module, "analyze", fake_analyze) - SceneAnalyzerNode().run("x", "llama3", 0.0, 60, system_prompt="CUSTOM") + SceneAnalyzerNode().run("x", system_prompt="CUSTOM") assert captured["system_prompt"] == "CUSTOM" +def test_system_prompt_override_wins_over_config(monkeypatch) -> None: + captured = {} + + def fake_analyze(description, backend, config): + captured["system_prompt"] = config.analyzer.system_prompt + captured["model"] = config.analyzer.model + return CompilerResult(data="S") + + monkeypatch.setattr(node_module, "analyze", fake_analyze) + config = Config.from_json({"analyzer": {"model": "mistral", "system_prompt": "FROM_CONFIG"}}) + SceneAnalyzerNode().run("x", config=config, system_prompt="OVERRIDE") + assert captured["system_prompt"] == "OVERRIDE" + assert captured["model"] == "mistral" + + def test_registered_for_comfyui() -> None: assert nodes.NODE_CLASS_MAPPINGS["SceneCompilerAnalyzer"] is SceneAnalyzerNode assert nodes.NODE_DISPLAY_NAME_MAPPINGS["SceneCompilerAnalyzer"] == "Scene Analyzer"