From 732c7d1a30b21c0588f818b025d4ddcf6e660765 Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Sat, 22 Aug 2026 12:16:44 -0400 Subject: [PATCH] fix(composer): an element the template cannot draw broke generation Reproduced from the review comment. A `group` element carrying minWidth generated: if width >= 64: # breakpoint: 64px+ displays only # manager.py.j2 wraps each element in the breakpoint and blink blocks, but the body comes from the per-type branches -- and a type with no branch contributes nothing, so the wrapper opens a block with no statements. ast.parse then fails and the caller is told only "Generated code has a syntax error: expected an indented block ... line 49", naming a line of generated source they never see. Two defences: - _preprocess_elements drops types the template has no branch for, alongside the existing `section` skip. This is the root cause: those elements should never have reached the template. - The branch chain ends in `{% else %}pass`, so a type added to the canvas before its drawing branch exists degrades to a no-op rather than a plugin that will not parse. The review also cited dynamic_text with binding_source != 'config'. That one does not reproduce -- the branch emits a draw_text regardless -- which is why an earlier attempt to reproduce this found nothing. _RENDERABLE_ELEMENT_TYPES has to stay in step with the template: a type listed with no branch emits an empty block again, and a branch missing from the list is silently dropped from every generated plugin. A test asserts the two sets are equal rather than trusting them to be maintained together. Tests: 12 new, covering group/unknown/section against breakpoint, blink and both nested, plus the set-equality and fallback checks. 7 fail with both defences reverted. 172 composer tests pass; full suite 3862 passed, the one failure being test_install_lowmem (pre-existing, awaiting #492). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW --- test/test_composer_empty_block.py | 80 +++++++++++++++++++ web_interface/blueprints/composer.py | 20 +++++ .../templates/v3/composer/manager.py.j2 | 2 + 3 files changed, 102 insertions(+) create mode 100644 test/test_composer_empty_block.py diff --git a/test/test_composer_empty_block.py b/test/test_composer_empty_block.py new file mode 100644 index 00000000..954d391b --- /dev/null +++ b/test/test_composer_empty_block.py @@ -0,0 +1,80 @@ +"""An element the template cannot draw must not produce an empty `if` block. + +manager.py.j2 wraps each element in `if width >= N:` (breakpoint) and/or +`if int(time.time() * 2) % 2:` (blink), and the body comes from the per-type +branches. A type with no branch contributed nothing, so the wrapper opened a +block with no statements in it. ast.parse in _generate_plugin_files then +failed and the caller was told only: + + Generated code has a syntax error: expected an indented block after + 'if' statement on line 49 + +which names a line of generated source the user never sees. Confirmed against +the code before the fix with a `group` element carrying minWidth. + +Two defences, both covered here: _preprocess_elements drops types the template +has no branch for, and the template emits a `pass` fallback so a type added to +the canvas before its branch exists degrades to a no-op instead of a broken +plugin. +""" +import re +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from web_interface.blueprints import composer as C # noqa: E402 + +TEMPLATE = (Path(__file__).resolve().parent.parent + / "web_interface/templates/v3/composer/manager.py.j2") + +BASE_META = {"id": "test-plugin", "name": "Clock", "author": "a", + "version": "1.0.0", "description": "d"} + + +def generate(element): + return C._generate_plugin_files({ + "metadata": BASE_META, + "elements": [element], + "dataModel": {"configVars": []}, + }) + + +@pytest.mark.parametrize("wrapper", [ + {"minWidth": 64}, # breakpoint block + {"blink": True}, # blink block + {"minWidth": 64, "blink": True}, # both, nested +]) +@pytest.mark.parametrize("etype", ["group", "widget_9000", "section"]) +def test_undrawable_element_does_not_break_generation(etype, wrapper): + element = {"type": etype, "x": 0, "y": 0, "color": "#ffffff", **wrapper} + files = generate(element) # must not raise ComposerInputError + assert "manager.py" in files + + +def test_drawable_element_still_renders_inside_a_breakpoint(): + files = generate({"type": "text", "text": "hi", "x": 0, "y": 0, + "minWidth": 64, "color": "#ffffff"}) + src = files["manager.py"] + assert "if width >= 64:" in src + assert "draw_text" in src + + +def test_renderable_types_match_the_template_branches(): + """The constant and the template must agree. + + A type listed in the constant with no branch emits an empty block (the bug + above); a type with a branch but missing from the constant is silently + dropped from every generated plugin. Neither is visible without this check. + """ + branches = set(re.findall(r"el\.type == '([a-z_]+)'", TEMPLATE.read_text())) + assert branches == set(C._RENDERABLE_ELEMENT_TYPES) + + +def test_template_closes_the_branch_chain_with_a_fallback(): + """Belt and braces: even if the constant drifts, no empty block escapes.""" + text = TEMPLATE.read_text() + assert "{% else %}" in text + assert "pass # element type" in text diff --git a/web_interface/blueprints/composer.py b/web_interface/blueprints/composer.py index ea6287b4..4c3f3d78 100644 --- a/web_interface/blueprints/composer.py +++ b/web_interface/blueprints/composer.py @@ -213,6 +213,16 @@ def _aligned_x_expr(x_base_expr: str, text_align: str, char_count: int, char_w: return x_base_expr +#: Element types manager.py.j2 has a drawing branch for. Kept next to the +#: preprocessor because the two must agree: a type here with no branch emits an +#: empty block, and a type with a branch but missing here is silently dropped. +_RENDERABLE_ELEMENT_TYPES = frozenset({ + 'text', 'dynamic_text', 'clock', 'countdown', 'rectangle', 'arc', + 'ellipse', 'pixel', 'rounded_rectangle', 'pips', 'sparkline', 'gauge', + 'marquee', 'progress_bar', +}) + + def _preprocess_elements(elements: list) -> list: """Expand raw element dicts into template-ready dicts with anchor expressions. @@ -231,6 +241,16 @@ def _preprocess_elements(elements: list) -> list: if t == 'section': continue + # A type the template has no branch for still gets its breakpoint and + # blink wrappers emitted, and those would open an `if` with nothing in + # it -- ast.parse then fails and the caller is told only "Generated + # code has a syntax error". Drop it here instead. The template also + # emits a `pass` fallback, so a type added to the canvas before its + # branch exists degrades to a no-op rather than a broken plugin. + if t not in _RENDERABLE_ELEMENT_TYPES: + logger.info("composer: skipping element type %r with no template branch", t) + continue + x_anchor = el.get('xAnchor') or None y_anchor = el.get('yAnchor') or None p['min_width'] = int(el.get('minWidth', 0) or 0) diff --git a/web_interface/templates/v3/composer/manager.py.j2 b/web_interface/templates/v3/composer/manager.py.j2 index 5d90e06d..87d9fa9a 100644 --- a/web_interface/templates/v3/composer/manager.py.j2 +++ b/web_interface/templates/v3/composer/manager.py.j2 @@ -288,6 +288,8 @@ class {{ class_name }}(BasePlugin): {{ pi }} [_pb_x, _pb_y, _pb_x + _pb_fill_w, _pb_y + {{ el.bar_height }}], {{ pi }} fill={{ el.fill_tuple }}, {{ pi }} ) +{% else %} +{{ pi }}pass # element type "{{ el.type }}" draws nothing {% endif %} {% endfor %} # ── End elements ───────────────────────────────────────────────