From b20684eeaa41ced63d0711702fecd6457fd3845e Mon Sep 17 00:00:00 2001 From: veguAI <152010387+vegu-ai-tools@users.noreply.github.com> Date: Wed, 22 Jul 2026 09:17:06 +0300 Subject: [PATCH] fix: ApplyStyle node crashed on any real style template id (#123) (#132) * fix: ApplyStyle node crashed on any real style template id (#123) * refactor: guard apply_style against non-visual templates, dedupe style resolution (#123 review) * refactor: consolidate remaining template-id resolutions onto find_template_by_id (#123) --- CHANGELOG.yaml | 1 + src/talemate/agents/visual/style.py | 65 +++++----- src/talemate/tale_mate.py | 20 +--- src/talemate/world_state/templates/base.py | 13 ++ tests/test_nodes_issue115.py | 131 +++++++++++++++++++-- 5 files changed, 175 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.yaml b/CHANGELOG.yaml index 51426c12..0d1dfbe6 100644 --- a/CHANGELOG.yaml +++ b/CHANGELOG.yaml @@ -38,6 +38,7 @@ - "Timeline Rollback: Previewing a revision could fail with an 'unhashable type' error when the scene's changelog contained deltas that diverged from the base snapshot — reconstruction now repairs the affected message history (and other list data) instead of crashing the timeline." - "Scene Export: Exporting a scene with 'Reset Progress' checked wiped the loaded scene's message history and world state and could break subsequent saves — the reset now applies only to the exported file, leaving the current session untouched." - "Client Editor: The Context Length input is now shown for every client type — previously clients without a self-hosted API URL (e.g. OpenRouter and the remote API clients) were missing it from the edit dialog, leaving the client-list slider as the only way to change their context token budget." + - "Node Editor: The Apply Style node crashed on any real style template id — it fed the id string into a code path expecting a visual type enum, so only the literal 'UNSPECIFIED' survived. The node now resolves the given template id directly and applies the full template. Note: passing 'UNSPECIFIED' as the template id no longer applies the configured art style — use the Apply Styles node for configuration-driven styles." - "Client Sliders: Adjusting the context or reasoning token budget sliders in the client list no longer jitters back and forth between values — stale client status echoes from the backend could snap the slider back to a previous value shortly after dragging. Saving a client also no longer triggers redundant status round-trips to the inference API, and quickly editing two clients back to back no longer drops the first client's pending change." - "Node Editor: The Clean Up Narration node crashed with an AttributeError whenever it ran, and the raise/Stop node errored with 'Unknown exception' when its StageExit choice was selected." - "Node Editor: The Compress Context ID Part node emitted the compressed value on its `uncompressed` output instead of the original part, and the As Bool node's error path itself crashed with a TypeError when a value could not be cast to a boolean." diff --git a/src/talemate/agents/visual/style.py b/src/talemate/agents/visual/style.py index 217ed8d4..5a68df87 100644 --- a/src/talemate/agents/visual/style.py +++ b/src/talemate/agents/visual/style.py @@ -14,6 +14,18 @@ __all__ = [ log = structlog.get_logger("talemate.agents.visual.style") +def _part_from_style(template: VisualStyle) -> VisualPromptPart: + """Build a VisualPromptPart from a style template, mapping the + template's keyword lists onto the part's raw keyword fields.""" + return VisualPromptPart( + positive_keywords_raw=template.positive_keywords, + negative_keywords_raw=template.negative_keywords, + positive_descriptive=template.positive_descriptive, + negative_descriptive=template.negative_descriptive, + instructions=template.instructions, + ) + + class StyleMixin: @classmethod def add_actions(cls, actions: dict[str, AgentAction]): @@ -121,11 +133,7 @@ class StyleMixin: if not template_id: return None - try: - group_uid, template_uid = template_id.split("__") - except ValueError: - return None - return templates.find_template(group_uid, template_uid) + return templates.find_template_by_id(template_id) def _get_current_art_style_name(self) -> str | None: """Get the name of the currently active art style template""" @@ -150,11 +158,26 @@ class StyleMixin: def apply_style( self, prompt: VisualPrompt, template_id: str ) -> VisualPromptPart | None: - template = self.style_template(template_id) - part = None - if template: - part = VisualPromptPart(**template.model_dump()) - prompt.parts.insert(0, part) + """Apply a specific style template (by `group_uid__template_uid`) to + the prompt, inserting it at the front of the part list. + + Returns the created part, or None if the template id is malformed or + the template was not found (prompt left unchanged). + """ + scene = getattr(self, "scene", None) + if not scene or not template_id: + return None + + manager: WorldStateManager = scene.world_state_manager + templates: Collection = manager.template_collection + + template = templates.find_template_by_id(template_id) + if not isinstance(template, VisualStyle): + # unknown id, or a valid id pointing at a non-visual template + return None + + part = _part_from_style(template) + prompt.parts.insert(0, part) return part def apply_styles(self, prompt: VisualPrompt, vis_type: VIS_TYPE) -> VisualPrompt: @@ -170,27 +193,9 @@ class StyleMixin: ) if template_subject_style: - prompt.parts.insert( - 0, - VisualPromptPart( - positive_keywords_raw=template_subject_style.positive_keywords, - negative_keywords_raw=template_subject_style.negative_keywords, - positive_descriptive=template_subject_style.positive_descriptive, - negative_descriptive=template_subject_style.negative_descriptive, - instructions=template_subject_style.instructions, - ), - ) + prompt.parts.insert(0, _part_from_style(template_subject_style)) if template_art_style: - prompt.parts.insert( - 0, - VisualPromptPart( - positive_keywords_raw=template_art_style.positive_keywords, - negative_keywords_raw=template_art_style.negative_keywords, - positive_descriptive=template_art_style.positive_descriptive, - negative_descriptive=template_art_style.negative_descriptive, - instructions=template_art_style.instructions, - ), - ) + prompt.parts.insert(0, _part_from_style(template_art_style)) return prompt diff --git a/src/talemate/tale_mate.py b/src/talemate/tale_mate.py index 2fdc8cb3..e38bd98e 100644 --- a/src/talemate/tale_mate.py +++ b/src/talemate/tale_mate.py @@ -463,14 +463,9 @@ class Scene(Emitter): if not self.writing_style_template: return None - try: - group_uid, template_uid = self.writing_style_template.split("__", 1) - # Ensure template collection is initialized via manager - return self.world_state_manager.template_collection.find_template( - group_uid, template_uid - ) - except ValueError: - return None + return self.world_state_manager.template_collection.find_template_by_id( + self.writing_style_template + ) def agent_persona(self, agent_name: str): """ @@ -480,14 +475,7 @@ class Scene(Emitter): uid = (self.agent_persona_templates or {}).get(agent_name) if not uid: return None - try: - group_uid, template_uid = uid.split("__", 1) - except ValueError: - return None - # Ensure template collection is initialized via manager - return self.world_state_manager.template_collection.find_template( - group_uid, template_uid - ) + return self.world_state_manager.template_collection.find_template_by_id(uid) @property def agent_persona_names(self) -> dict[str, str]: diff --git a/src/talemate/world_state/templates/base.py b/src/talemate/world_state/templates/base.py index 83771cf4..6b2e5a55 100644 --- a/src/talemate/world_state/templates/base.py +++ b/src/talemate/world_state/templates/base.py @@ -436,6 +436,19 @@ class Collection(pydantic.BaseModel): return group.find(template_uid) return None + def find_template_by_id(self, template_id: str) -> Template | None: + """Resolve a `group_uid__template_uid` id to a template. + + Returns None for empty/malformed ids or when no template matches. + """ + if not template_id: + return None + try: + group_uid, template_uid = template_id.split("__", 1) + except ValueError: + return None + return self.find_template(group_uid, template_uid) + def remove(self, group: Group, save: bool = True): existing = self.find(group.uid) if existing is None: diff --git a/tests/test_nodes_issue115.py b/tests/test_nodes_issue115.py index e19ae004..2ed5c7e8 100644 --- a/tests/test_nodes_issue115.py +++ b/tests/test_nodes_issue115.py @@ -40,7 +40,9 @@ from talemate.game.engine.nodes.world_state import GenerationOptions, Spices from talemate.agents.editor.nodes import CleanUpNarration from talemate.agents.director.auto_direct_nodes import GenerateSceneTypes from talemate.agents.visual.nodes import ApplyStyle, ApplyStyles -from talemate.agents.visual.schema import VisualPrompt +from talemate.agents.visual.schema import VisualPrompt, VisualPromptPart +from talemate.world_state.templates.base import Template +from talemate.world_state.templates.visual import VisualStyle from talemate.agents.world_state.nodes import StateReinforcement from talemate.scene.schema import SceneType from talemate.game import focal as focal_module @@ -342,16 +344,43 @@ async def test_apply_styles_state_passthrough(mock_scene): assert isinstance(captured["prompt"], VisualPrompt) -@pytest.mark.asyncio -async def test_apply_style_state_passthrough(mock_scene): - captured = {} +class _FakeTemplateCollection: + """Minimal stand-in for the world-state template Collection, keyed by + (group_uid, template_uid).""" + + def __init__(self, templates: dict): + self.templates = templates + + def find_template(self, group_uid, template_uid): + return self.templates.get((group_uid, template_uid)) + + def find_template_by_id(self, template_id): + if not template_id: + return None + try: + group_uid, template_uid = template_id.split("__", 1) + except ValueError: + return None + return self.find_template(group_uid, template_uid) + + +def _digital_art_template() -> VisualStyle: + return VisualStyle( + name="Digital Art", + uid="digital_art", + group="visual_styles", + positive_keywords=["masterpiece", "vibrant colors"], + negative_keywords=["blurry"], + positive_descriptive="digital painting", + instructions="Make it pop", + ) + + +def _run_apply_style_graph(captured, template_id): node = ApplyStyle() prompt = VisualPrompt() - # NOTE: agent.apply_style crashes on any template_id other than - # "UNSPECIFIED" (it feeds the id string into style_template(), which - # expects a VIS_TYPE) - pre-existing bug outside issue #115's scope. - const = make_constant(state="marker", prompt=prompt, template_id="UNSPECIFIED") - capture = make_capture(captured, "state", "prompt", "prompt_part") + const = make_constant(state="marker", prompt=prompt, template_id=template_id) + capture = make_capture(captured, "state", "prompt", "template_id", "prompt_part") graph = build_graph(const, node, capture) graph.connect(const.get_output_socket("state"), node.get_input_socket("state")) @@ -361,14 +390,98 @@ async def test_apply_style_state_passthrough(mock_scene): ) graph.connect(node.get_output_socket("state"), capture.get_input_socket("state")) graph.connect(node.get_output_socket("prompt"), capture.get_input_socket("prompt")) + graph.connect( + node.get_output_socket("template_id"), capture.get_input_socket("template_id") + ) graph.connect( node.get_output_socket("prompt_part"), capture.get_input_socket("prompt_part") ) + return graph, prompt + + +@pytest.mark.asyncio +async def test_apply_style_state_passthrough(mock_scene): + """ApplyStyle passes state, prompt and template_id through (issue #115 + socket regression coverage, now exercising a real template id after the + #123 fix).""" + mock_scene._world_state_templates = _FakeTemplateCollection( + {("visual_styles", "digital_art"): _digital_art_template()} + ) + captured = {} + graph, prompt = _run_apply_style_graph(captured, "visual_styles__digital_art") await execute_graph(mock_scene, graph) assert captured["state"] == "marker" assert captured["prompt"] is prompt + assert captured["template_id"] == "visual_styles__digital_art" + + +@pytest.mark.asyncio +async def test_apply_style_resolves_template_part(mock_scene): + """apply_style crashed on any real template id (str fed into the + VIS_TYPE-expecting style_template); it now resolves the template directly + via find_template and inserts a part built from it at the front of the + prompt's part list (issue #123).""" + mock_scene._world_state_templates = _FakeTemplateCollection( + {("visual_styles", "digital_art"): _digital_art_template()} + ) + captured = {} + graph, prompt = _run_apply_style_graph(captured, "visual_styles__digital_art") + + await execute_graph(mock_scene, graph) + + part = captured["prompt_part"] + assert isinstance(part, VisualPromptPart) + assert prompt.parts[0] is part + assert part.positive_keywords_raw == ["masterpiece", "vibrant colors"] + assert part.negative_keywords_raw == ["blurry"] + assert part.positive_descriptive == "digital painting" + assert part.instructions == "Make it pop" + + +@pytest.mark.asyncio +async def test_apply_style_wrong_template_type_no_crash(mock_scene): + """A valid id pointing at a non-visual template (e.g. a writing style) + must degrade to no part instead of crashing on the missing keyword + fields (PR #132 review).""" + mock_scene._world_state_templates = _FakeTemplateCollection( + {("writing_styles", "flowery"): Template(name="Flowery")} + ) + captured = {} + graph, prompt = _run_apply_style_graph(captured, "writing_styles__flowery") + + await execute_graph(mock_scene, graph) + + assert captured["prompt_part"] is None + assert prompt.parts == [] + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "template_id", + [ + "visual_styles__nonexistent", # unknown template + "nonexistent_group__digital_art", # unknown group + "UNSPECIFIED", # not a template id (former special case, #123) + "no-separator", # malformed + "", # empty + ], +) +async def test_apply_style_unknown_template_id_no_crash(mock_scene, template_id): + """Unknown, malformed or empty template ids resolve to no part and leave + the prompt unchanged instead of crashing (issue #123).""" + mock_scene._world_state_templates = _FakeTemplateCollection( + {("visual_styles", "digital_art"): _digital_art_template()} + ) + captured = {} + graph, prompt = _run_apply_style_graph(captured, template_id) + + await execute_graph(mock_scene, graph) + + assert captured["prompt_part"] is None + assert prompt.parts == [] + assert captured["prompt"] is prompt @pytest.mark.asyncio