diff --git a/README.md b/README.md index 842592b..026d308 100644 --- a/README.md +++ b/README.md @@ -82,15 +82,14 @@ fixed workflow — every node can be used independently. | Node | Input | Output | |---|---|---| -| **Scene Analyzer** | Natural language, model, temperature, timeout | Scene JSON (+ warnings, errors, raw) | -| **Scene Validator** | Scene JSON | Validated Scene JSON (+ warnings, errors, raw) | -| **Resolver** | Scene JSON, Knowledge Base | Prompt (+ warnings, errors, json) | +| **Scene Analyzer** | Natural language, Config (optional) | Scene JSON (+ warnings, errors, raw) | +| **Scene Validator** | Scene JSON, Config (optional) | Validated Scene JSON (+ warnings, errors, raw) | +| **Resolver** | Scene JSON, Knowledge Base, Config (optional) | Prompt (+ warnings, errors, json) | ### Support nodes - **Debug Viewer** — inspect any intermediate state (Scene JSON, Resolved Tags, categories, warnings, errors). -- **Knowledge Base Loader** — load the active Knowledge Base, with manual reload for development. -- **Configuration Node** — centralize compiler settings without editing workflows. +- **Configuration Node** — the single place for compiler settings; it also loads the Knowledge Base from the configured path and emits it (as a `knowledge_base` output) for the Resolver, so there is no separate loader node. ### Knowledge Base Editor (optional web tool) diff --git a/docs/nodes.md b/docs/nodes.md index 97d6be2..d5c0f45 100644 --- a/docs/nodes.md +++ b/docs/nodes.md @@ -31,16 +31,16 @@ 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), `version` (STRING, optional), `config` (COMPILER_CONFIG, optional — its KB path/version win) | `knowledge_base` (KNOWLEDGE_BASE), `warnings` (STRING), `errors` (STRING) | +| **Configuration** | one input per configuration option (analyzer, resolver, validator, prompt builder, semantic, debug), plus the Knowledge Base `knowledge_base` path, `resolver_knowledge_base_version`, and `knowledge_base_reload` | `config` (COMPILER_CONFIG), `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 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 **Configuration** node is the single place for compiler settings. Its `config` + output feeds the optional `config` input of the Scene Analyzer, Validator, and + Resolver, and it also **loads the Knowledge Base** from the configured path/version + and emits it on its `knowledge_base` output (wired into the Resolver's + `knowledge_base` input) — so the Knowledge Base directory is entered in one place. + There is no separate Knowledge Base Loader node. Bump `knowledge_base_reload` to + force a fresh read (no automatic file watching). - The **Debug Viewer** is read-only; connect any intermediate state to inspect it. A ready-to-use workflow wiring all of these is in diff --git a/examples/workflows/scene_compiler_pipeline.json b/examples/workflows/scene_compiler_pipeline.json index 44c099d..737791f 100644 --- a/examples/workflows/scene_compiler_pipeline.json +++ b/examples/workflows/scene_compiler_pipeline.json @@ -1,53 +1,38 @@ { "last_node_id": 5, - "last_link_id": 7, + "last_link_id": 6, "nodes": [ { "id": 1, "type": "SceneCompilerConfiguration", "pos": [40, 40], - "size": [300, 560], + "size": [300, 600], "flags": {}, "order": 0, "mode": 0, "inputs": [], "outputs": [ - { "name": "config", "type": "COMPILER_CONFIG", "links": [1, 2, 7], "slot_index": 0 }, - { "name": "errors", "type": "STRING", "links": null, "slot_index": 1 } + { "name": "config", "type": "COMPILER_CONFIG", "links": [1, 2, 3], "slot_index": 0 }, + { "name": "knowledge_base", "type": "KNOWLEDGE_BASE", "links": [4], "slot_index": 1 }, + { "name": "warnings", "type": "STRING", "links": null, "slot_index": 2 }, + { "name": "errors", "type": "STRING", "links": null, "slot_index": 3 } ], "properties": { "Node name for S&R": "SceneCompilerConfiguration" }, "widgets_values": [ "llama3", 0.0, 3, 300, "knowledge_base/", true, true, true, 8, false, false, - "easy_illustrious", ",", true, false, "basic", "", "", false, 0.5, "char_ngram" + "easy_illustrious", ",", true, false, "basic", "", "", false, 0.5, "char_ngram", 0 ] }, - { - "id": 2, - "type": "SceneCompilerKnowledgeBaseLoader", - "pos": [40, 640], - "size": [300, 100], - "flags": {}, - "order": 1, - "mode": 0, - "inputs": [], - "outputs": [ - { "name": "knowledge_base", "type": "KNOWLEDGE_BASE", "links": [4], "slot_index": 0 }, - { "name": "warnings", "type": "STRING", "links": null, "slot_index": 1 }, - { "name": "errors", "type": "STRING", "links": null, "slot_index": 2 } - ], - "properties": { "Node name for S&R": "SceneCompilerKnowledgeBaseLoader" }, - "widgets_values": ["knowledge_base/", 0, ""] - }, { "id": 3, "type": "SceneCompilerAnalyzer", "pos": [380, 40], "size": [340, 200], "flags": {}, - "order": 2, + "order": 1, "mode": 0, "inputs": [ - { "name": "config", "type": "COMPILER_CONFIG", "link": 7 } + { "name": "config", "type": "COMPILER_CONFIG", "link": 3 } ], "outputs": [ { "name": "scene", "type": "SCENE", "links": [5], "slot_index": 0 }, @@ -67,7 +52,7 @@ "pos": [760, 40], "size": [280, 120], "flags": {}, - "order": 3, + "order": 2, "mode": 0, "inputs": [ { "name": "scene", "type": "SCENE", "link": 5 }, @@ -88,7 +73,7 @@ "pos": [1060, 40], "size": [300, 160], "flags": {}, - "order": 4, + "order": 3, "mode": 0, "inputs": [ { "name": "scene", "type": "SCENE", "link": 6 }, @@ -108,10 +93,10 @@ "links": [ [1, 1, 0, 4, 1, "COMPILER_CONFIG"], [2, 1, 0, 5, 2, "COMPILER_CONFIG"], - [4, 2, 0, 5, 1, "KNOWLEDGE_BASE"], + [3, 1, 0, 3, 0, "COMPILER_CONFIG"], + [4, 1, 1, 5, 1, "KNOWLEDGE_BASE"], [5, 3, 0, 4, 0, "SCENE"], - [6, 4, 0, 5, 0, "SCENE"], - [7, 1, 0, 3, 0, "COMPILER_CONFIG"] + [6, 4, 0, 5, 0, "SCENE"] ], "groups": [], "config": {}, diff --git a/nodes/__init__.py b/nodes/__init__.py index 61dfa32..bc67b4b 100644 --- a/nodes/__init__.py +++ b/nodes/__init__.py @@ -9,7 +9,6 @@ from __future__ import annotations from .configuration_node import ConfigurationNode from .debug_viewer_node import DebugViewerNode -from .knowledge_base_loader_node import KnowledgeBaseLoaderNode from .resolver_node import ResolverNode from .scene_analyzer_node import SceneAnalyzerNode from .scene_validator_node import SceneValidatorNode @@ -19,7 +18,6 @@ NODE_CLASS_MAPPINGS: dict[str, type] = { "SceneCompilerValidator": SceneValidatorNode, "SceneCompilerResolver": ResolverNode, "SceneCompilerDebugViewer": DebugViewerNode, - "SceneCompilerKnowledgeBaseLoader": KnowledgeBaseLoaderNode, "SceneCompilerConfiguration": ConfigurationNode, } @@ -28,7 +26,6 @@ NODE_DISPLAY_NAME_MAPPINGS: dict[str, str] = { "SceneCompilerValidator": "Scene Validator", "SceneCompilerResolver": "Resolver", "SceneCompilerDebugViewer": "Debug Viewer", - "SceneCompilerKnowledgeBaseLoader": "Knowledge Base Loader", "SceneCompilerConfiguration": "Configuration", } diff --git a/nodes/configuration_node.py b/nodes/configuration_node.py index bc8840b..1dde1c4 100644 --- a/nodes/configuration_node.py +++ b/nodes/configuration_node.py @@ -1,8 +1,10 @@ """ComfyUI node for the compiler Configuration (MASTER_SPEC §20, §23). -A thin interface: it collects the configuration options as node inputs and emits -a single COMPILER_CONFIG that the other nodes consume, so behaviour can change -without editing the workflow. No compiler logic; it never imports ComfyUI. +A thin interface: it collects the configuration options as node inputs and emits a +single COMPILER_CONFIG that the other nodes consume, so behaviour can change without +editing the workflow. It also loads the Knowledge Base from the configured path and +emits it for the Resolver, so the Knowledge Base directory is entered in exactly one +place (there is no separate Knowledge Base Loader node). It never imports ComfyUI. """ from __future__ import annotations @@ -12,17 +14,18 @@ from typing import Any from compiler.common.config import Config, ConfigError from .adapters import format_messages +from .kb_loading import load_cached_knowledge_base _DEBUG_LEVELS = ["none", "basic", "verbose", "developer"] class ConfigurationNode: - """Centralizes compiler configuration and emits it for the other nodes.""" + """Centralizes compiler configuration and loads the Knowledge Base for the pipeline.""" CATEGORY = "Scene Compiler" FUNCTION = "run" - RETURN_TYPES = ("COMPILER_CONFIG", "STRING") - RETURN_NAMES = ("config", "errors") + RETURN_TYPES = ("COMPILER_CONFIG", "KNOWLEDGE_BASE", "STRING", "STRING") + RETURN_NAMES = ("config", "knowledge_base", "warnings", "errors") @classmethod def INPUT_TYPES(cls) -> dict[str, Any]: @@ -187,6 +190,14 @@ class ConfigurationNode: "tooltip": "Embedding backend for the semantic fallback (offline).", }, ), + "knowledge_base_reload": ( + "INT", + { + "default": 0, + "min": 0, + "tooltip": "Bump to force a fresh read of the Knowledge Base from disk.", + }, + ), }, } @@ -213,7 +224,8 @@ class ConfigurationNode: semantic_enabled: bool = False, semantic_min_similarity: float = 0.5, semantic_backend: str = "char_ngram", - ) -> tuple[Any, str]: + knowledge_base_reload: int = 0, + ) -> tuple[Any, Any, str, str]: analyzer: dict[str, Any] = { "model": analyzer_model, "temperature": analyzer_temperature, @@ -255,5 +267,13 @@ class ConfigurationNode: try: config = Config.from_json(document) except ConfigError as error: - return (None, format_messages([error.message])) - return (config, "") + return (None, None, "", format_messages([error.message])) + + # Load the Knowledge Base from the configured path/version and emit it for + # the Resolver, so the Knowledge Base directory is entered in one place. + kb, warnings, errors, _raw = load_cached_knowledge_base( + knowledge_base, + resolver_knowledge_base_version, + knowledge_base_reload, + ) + return (config, kb, warnings, errors) diff --git a/nodes/kb_loading.py b/nodes/kb_loading.py new file mode 100644 index 0000000..250ac83 --- /dev/null +++ b/nodes/kb_loading.py @@ -0,0 +1,77 @@ +"""Shared Knowledge Base loading for the Scene Compiler nodes. + +Loads a Knowledge Base directory with a process-wide load-once cache keyed by +``(resolved_path, version)`` so the (large) Knowledge Base is read once and reused +across node executions (#130); bumping ``reload`` forces a fresh read. A relative +path is resolved against this package (where the shipped ``knowledge_base/`` lives), +not ComfyUI's working directory, so the default works regardless of where ComfyUI +is launched from. + +The Configuration node uses this to emit the Knowledge Base for the Resolver; no +compiler logic lives here beyond delegating to the loader module, and it never +imports ComfyUI. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +from compiler.common.knowledge_base import ( + KnowledgeBaseError, + KnowledgeBaseLoader, +) + +from .adapters import format_messages + +# Package root: the repository directory that ships knowledge_base/, prompts/, ... +_PACKAGE_ROOT = Path(__file__).resolve().parent.parent + +# Process-wide cache keyed by the resolved path and pinned version; the ``reload`` +# counter forces a fresh load when bumped. +_LOADERS: dict[tuple[str, str], KnowledgeBaseLoader] = {} +_LAST_RELOAD: dict[tuple[str, str], int] = {} + + +def resolve_kb_path(path: str) -> Path: + """Resolve a relative Knowledge Base path against the package root.""" + candidate = Path(path) + return candidate if candidate.is_absolute() else _PACKAGE_ROOT / candidate + + +def load_cached_knowledge_base( + path: str, version: str = "", reload: int = 0 +) -> tuple[Any, str, str, str]: + """Load (and cache) the Knowledge Base, returning ``(kb, warnings, errors, raw)``. + + ``kb`` is ``None`` on a load failure. The cache is reused across calls with the + same ``(path, version)`` unless ``reload`` changes. + """ + resolved = resolve_kb_path(path) + requested_version = version or None + key = (str(resolved), version) + loader = _LOADERS.get(key) + if loader is None: + loader = KnowledgeBaseLoader(resolved, requested_version=requested_version) + _LOADERS[key] = loader + _LAST_RELOAD[key] = reload + try: + if reload != _LAST_RELOAD[key]: + _LAST_RELOAD[key] = reload + knowledge_base = loader.reload() + else: + knowledge_base = loader.get() + except KnowledgeBaseError as error: + errors = format_messages([error.message, *error.findings]) + return (None, "", errors, f"Failed to load Knowledge Base from: {resolved}") + + count = len(knowledge_base) + warnings = "" + if count == 0: + warnings = ( + f"Knowledge Base at '{resolved}' is empty (0 entries); " + "check the path. Every concept will be reported as unknown." + ) + sample = ", ".join(sorted(knowledge_base.by_id)[:20]) + raw = f"Loaded {count} entries from {resolved}\nFirst entries: {sample}" + return (knowledge_base, warnings, "", raw) diff --git a/nodes/knowledge_base_loader_node.py b/nodes/knowledge_base_loader_node.py deleted file mode 100644 index fe49c6d..0000000 --- a/nodes/knowledge_base_loader_node.py +++ /dev/null @@ -1,141 +0,0 @@ -"""ComfyUI node for the Knowledge Base Loader (MASTER_SPEC §20). - -A thin interface: it loads a Knowledge Base directory via the loader module and -exposes it for the Resolver node. No compiler logic; it never imports ComfyUI. - -A relative ``path`` is resolved against this package (where the shipped -``knowledge_base/`` lives), not ComfyUI's working directory, so the default works -regardless of where ComfyUI is launched from. Reloading is explicit: bumping the -``reload`` input re-executes the node (there is no automatic file watching). -""" - -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, -) - -from .adapters import format_messages - -# Package root: the repository directory that ships knowledge_base/, prompts/, ... -_PACKAGE_ROOT = Path(__file__).resolve().parent.parent - -# Process-wide Knowledge Base cache so the (large) KB loads once per (path, version) -# and is reused across node executions instead of being re-read on every run (#130). -# Keyed by the resolved path and pinned version; the ``reload`` widget forces a -# fresh load when bumped. -_LOADERS: dict[tuple[str, str], KnowledgeBaseLoader] = {} -_LAST_RELOAD: dict[tuple[str, str], int] = {} - - -def _resolve_path(path: str) -> Path: - """Resolve a relative Knowledge Base path against the package root.""" - candidate = Path(path) - return candidate if candidate.is_absolute() else _PACKAGE_ROOT / candidate - - -class KnowledgeBaseLoaderNode: - """Loads the Knowledge Base and exposes it for the Resolver.""" - - CATEGORY = "Scene Compiler" - FUNCTION = "run" - RETURN_TYPES = ("KNOWLEDGE_BASE", "STRING", "STRING", "STRING") - RETURN_NAMES = ("knowledge_base", "warnings", "errors", "raw") - - @classmethod - def INPUT_TYPES(cls) -> dict[str, Any]: - return { - "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, - "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 = "", - 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) - loader = _LOADERS.get(key) - if loader is None: - loader = KnowledgeBaseLoader(resolved, requested_version=requested_version) - _LOADERS[key] = loader - _LAST_RELOAD[key] = reload - try: - # Bumping the ``reload`` widget forces a fresh read; otherwise the cached - # Knowledge Base is reused across runs. - if reload != _LAST_RELOAD[key]: - _LAST_RELOAD[key] = reload - knowledge_base = loader.reload() - else: - knowledge_base = loader.get() - except KnowledgeBaseError as error: - errors = format_messages([error.message, *error.findings]) - return (None, "", errors, f"Failed to load Knowledge Base from: {resolved}") - - count = len(knowledge_base) - warnings = "" - if count == 0: - warnings = ( - f"Knowledge Base at '{resolved}' is empty (0 entries); " - "check the path. Every concept will be reported as unknown." - ) - sample = ", ".join(sorted(knowledge_base.by_id)[:20]) - raw = f"Loaded {count} entries from {resolved}\nFirst entries: {sample}" - return (knowledge_base, warnings, "", raw) diff --git a/tests/test_configuration_node.py b/tests/test_configuration_node.py index 0ae1326..f2f244d 100644 --- a/tests/test_configuration_node.py +++ b/tests/test_configuration_node.py @@ -4,6 +4,7 @@ from __future__ import annotations import nodes from compiler.common.config import Config +from compiler.common.knowledge_base import KnowledgeBase from nodes.configuration_node import ConfigurationNode @@ -32,19 +33,29 @@ def default_inputs() -> dict: def test_node_metadata() -> None: - assert ConfigurationNode.RETURN_TYPES == ("COMPILER_CONFIG", "STRING") - assert ConfigurationNode.RETURN_NAMES == ("config", "errors") + assert ConfigurationNode.RETURN_TYPES == ( + "COMPILER_CONFIG", + "KNOWLEDGE_BASE", + "STRING", + "STRING", + ) + assert ConfigurationNode.RETURN_NAMES == ("config", "knowledge_base", "warnings", "errors") required = ConfigurationNode.INPUT_TYPES()["required"] assert "analyzer_model" in required assert "debug_level" in required - assert "analyzer_system_prompt" in ConfigurationNode.INPUT_TYPES()["optional"] + optional = ConfigurationNode.INPUT_TYPES()["optional"] + assert "analyzer_system_prompt" in optional + assert "knowledge_base_reload" in optional -def test_default_inputs_produce_default_config() -> None: - config, errors = ConfigurationNode().run(**default_inputs()) +def test_default_inputs_produce_default_config_and_load_the_kb() -> None: + config, kb, warnings, errors = ConfigurationNode().run(**default_inputs()) assert isinstance(config, Config) assert config == Config() - assert errors == "" + # The node now also loads the shipped reference Knowledge Base. + assert isinstance(kb, KnowledgeBase) + assert len(kb) > 100 + assert (warnings, errors) == ("", "") def test_inputs_flow_into_config() -> None: @@ -56,7 +67,7 @@ def test_inputs_flow_into_config() -> None: inputs["prompt_separator"] = " | " inputs["debug_enabled"] = True inputs["debug_level"] = "verbose" - config, _ = ConfigurationNode().run(**inputs) + config, _kb, _warnings, _errors = ConfigurationNode().run(**inputs) assert config.analyzer.model == "mistral" assert config.resolver.max_expansion_depth == 4 assert config.resolver.include_nsfw is True @@ -66,13 +77,24 @@ def test_inputs_flow_into_config() -> None: assert config.debug.level == "verbose" +def test_invalid_knowledge_base_path_surfaces_error_but_keeps_config() -> None: + inputs = default_inputs() + inputs["knowledge_base"] = "does/not/exist/" + config, kb, warnings, errors = ConfigurationNode().run(**inputs) + # The config is still emitted (the Analyzer/Validator do not need the KB); the + # empty/missing Knowledge Base is surfaced as a warning. + assert isinstance(config, Config) + assert len(kb) == 0 + assert "empty" in warnings.lower() + + def test_system_prompt_override_is_applied() -> None: - config, _ = ConfigurationNode().run(**default_inputs(), analyzer_system_prompt="CUSTOM") + config, *_ = ConfigurationNode().run(**default_inputs(), analyzer_system_prompt="CUSTOM") assert config.analyzer.system_prompt == "CUSTOM" def test_empty_system_prompt_is_omitted() -> None: - config, _ = ConfigurationNode().run(**default_inputs(), analyzer_system_prompt="") + config, *_ = ConfigurationNode().run(**default_inputs(), analyzer_system_prompt="") assert config.analyzer.system_prompt is None @@ -81,7 +103,7 @@ def test_debug_level_options_match_config_schema() -> None: for level in options: inputs = default_inputs() inputs["debug_level"] = level - config, errors = ConfigurationNode().run(**inputs) + config, _kb, _warnings, errors = ConfigurationNode().run(**inputs) assert isinstance(config, Config) assert errors == "" diff --git a/tests/test_examples.py b/tests/test_examples.py index 91c1d88..055aed1 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -49,7 +49,6 @@ def test_workflow_is_valid_json_referencing_registered_nodes() -> None: # The full pipeline is present. for required in ( "SceneCompilerConfiguration", - "SceneCompilerKnowledgeBaseLoader", "SceneCompilerAnalyzer", "SceneCompilerValidator", "SceneCompilerResolver", diff --git a/tests/test_kb_loading.py b/tests/test_kb_loading.py new file mode 100644 index 0000000..6e64c55 --- /dev/null +++ b/tests/test_kb_loading.py @@ -0,0 +1,51 @@ +"""Tests for the shared node Knowledge Base loading helper (was issue #25).""" + +from __future__ import annotations + +import json + +from compiler.common.knowledge_base import KnowledgeBase +from nodes.kb_loading import load_cached_knowledge_base + + +def write_kb(directory, entries: list[dict]) -> str: + (directory / "hair.json").write_text(json.dumps(entries), encoding="utf-8") + return str(directory) + + +def test_loads_knowledge_base(tmp_path) -> None: + path = write_kb(tmp_path, [{"id": "long_hair", "tags": ["long hair"], "category": "hair"}]) + kb, warnings, errors, *_ = load_cached_knowledge_base(path) + assert isinstance(kb, KnowledgeBase) + assert kb.get("long_hair") is not None + assert (warnings, errors) == ("", "") + + +def test_invalid_knowledge_base_surfaces_sc0004(tmp_path) -> None: + path = write_kb(tmp_path, [{"id": "x", "tags": ["x"], "category": "nope"}]) + kb, warnings, errors, *_ = load_cached_knowledge_base(path) + assert kb is None + assert "SC0004" in errors + + +def test_relative_path_resolves_against_package(monkeypatch, tmp_path) -> None: + # Even with an unrelated working directory (as under ComfyUI), the default + # relative path must load the shipped Knowledge Base, not an empty one. + monkeypatch.chdir(tmp_path) + kb, warnings, errors, raw = load_cached_knowledge_base("knowledge_base/") + assert isinstance(kb, KnowledgeBase) + assert len(kb) > 100 + assert (warnings, errors) == ("", "") + assert "entries" in raw + + +def test_empty_knowledge_base_warns(tmp_path) -> None: + kb, warnings, errors, raw = load_cached_knowledge_base(str(tmp_path)) + assert len(kb) == 0 + assert "empty" in warnings.lower() + + +def test_reload_input_is_accepted(tmp_path) -> None: + path = write_kb(tmp_path, [{"id": "long_hair", "tags": ["long hair"], "category": "hair"}]) + kb, _, _, _ = load_cached_knowledge_base(path, reload=5) + assert isinstance(kb, KnowledgeBase) diff --git a/tests/test_knowledge_base_loader_node.py b/tests/test_knowledge_base_loader_node.py deleted file mode 100644 index a88bd69..0000000 --- a/tests/test_knowledge_base_loader_node.py +++ /dev/null @@ -1,82 +0,0 @@ -"""Tests for the Knowledge Base Loader ComfyUI node (issue #25).""" - -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 - - -def write_kb(directory, entries: list[dict]) -> str: - (directory / "hair.json").write_text(json.dumps(entries), encoding="utf-8") - return str(directory) - - -def test_node_metadata() -> None: - assert KnowledgeBaseLoaderNode.RETURN_TYPES == ("KNOWLEDGE_BASE", "STRING", "STRING", "STRING") - assert KnowledgeBaseLoaderNode.RETURN_NAMES == ("knowledge_base", "warnings", "errors", "raw") - inputs = KnowledgeBaseLoaderNode.INPUT_TYPES() - assert "path" in inputs["required"] - assert "reload" in inputs["optional"] - - -def test_loads_knowledge_base(tmp_path) -> None: - path = write_kb(tmp_path, [{"id": "long_hair", "tags": ["long hair"], "category": "hair"}]) - kb, warnings, errors, *_ = KnowledgeBaseLoaderNode().run(path) - assert isinstance(kb, KnowledgeBase) - assert kb.get("long_hair") is not None - assert (warnings, errors) == ("", "") - - -def test_invalid_knowledge_base_surfaces_sc0004(tmp_path) -> None: - path = write_kb(tmp_path, [{"id": "x", "tags": ["x"], "category": "nope"}]) - kb, warnings, errors, *_ = KnowledgeBaseLoaderNode().run(path) - assert kb is None - assert "SC0004" in errors - - -def test_relative_path_resolves_against_package(monkeypatch, tmp_path) -> None: - # Even with an unrelated working directory (as under ComfyUI), the default - # relative path must load the shipped Knowledge Base, not an empty one. - monkeypatch.chdir(tmp_path) - kb, warnings, errors, raw = KnowledgeBaseLoaderNode().run("knowledge_base/") - assert isinstance(kb, KnowledgeBase) - assert len(kb) > 100 - assert (warnings, errors) == ("", "") - assert "entries" in raw - - -def test_empty_knowledge_base_warns(tmp_path) -> None: - kb, warnings, errors, raw = KnowledgeBaseLoaderNode().run(str(tmp_path)) - assert len(kb) == 0 - assert "empty" in warnings.lower() - - -def test_reload_input_is_accepted(tmp_path) -> None: - path = write_kb(tmp_path, [{"id": "long_hair", "tags": ["long hair"], "category": "hair"}]) - kb, _, _, _ = KnowledgeBaseLoaderNode().run(path, reload=5) - 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 ( - nodes.NODE_DISPLAY_NAME_MAPPINGS["SceneCompilerKnowledgeBaseLoader"] - == "Knowledge Base Loader" - ) diff --git a/tests/test_performance.py b/tests/test_performance.py index f7078aa..e2f13e8 100644 --- a/tests/test_performance.py +++ b/tests/test_performance.py @@ -103,8 +103,8 @@ def test_pipeline_does_not_reload_knowledge_base(monkeypatch) -> None: assert calls["n"] == 0 # compilation reuses the passed Knowledge Base -def test_loader_node_reuses_cached_kb_across_runs(tmp_path, monkeypatch) -> None: - from nodes import knowledge_base_loader_node as loader_node +def test_kb_loading_reuses_cached_kb_across_runs(tmp_path, monkeypatch) -> None: + from nodes import kb_loading (tmp_path / "hair.json").write_text( '[{"id": "long_hair", "tags": ["long hair"], "category": "hair"}]', encoding="utf-8" @@ -118,16 +118,15 @@ def test_loader_node_reuses_cached_kb_across_runs(tmp_path, monkeypatch) -> None monkeypatch.setattr(kb_module, "load_knowledge_base", counting_load) # Isolate the process-wide cache for this test. - monkeypatch.setattr(loader_node, "_LOADERS", {}) - monkeypatch.setattr(loader_node, "_LAST_RELOAD", {}) + monkeypatch.setattr(kb_loading, "_LOADERS", {}) + monkeypatch.setattr(kb_loading, "_LAST_RELOAD", {}) - node = loader_node.KnowledgeBaseLoaderNode() - kb1 = node.run(str(tmp_path))[0] - kb2 = node.run(str(tmp_path))[0] - assert kb1 is kb2 # same cached object reused across run() calls + kb1 = kb_loading.load_cached_knowledge_base(str(tmp_path))[0] + kb2 = kb_loading.load_cached_knowledge_base(str(tmp_path))[0] + assert kb1 is kb2 # same cached object reused across calls assert calls["n"] == 1 # loaded once - kb3 = node.run(str(tmp_path), reload=1)[0] # bumping reload forces a fresh read + kb3 = kb_loading.load_cached_knowledge_base(str(tmp_path), reload=1)[0] # forces a fresh read assert calls["n"] == 2 assert kb3 is not kb1