From fe3f8154b51c9291ba489b95bc75a7399daabfb0 Mon Sep 17 00:00:00 2001 From: Charles Yao Date: Sun, 20 Sep 2026 23:08:27 -0700 Subject: [PATCH] feat(dify-builder): carry node usage rules on the Builder-only directive Meeting item 9 asked for per-node usage detail in the generator prompts. Those prompts are shared with cmd+K, so the detail rides the channel the Builder already owns instead: _WORKFLOW_TOPOLOGY_DIRECTIVE, prepended to every generator instruction at build.py:377 and invisible to cmd+K. That mechanism is proven -- it is why workflow mode never gets 'answer' nodes. The rules added are the ones that broke real builds: tool nodes expose text/files/json and not 'result' (ESQ1-296, ESQ1-289), document-extractor needs a file input, container children belong inside the container (ESQ1-288), if-else operators must match the variable's type (ESQ1-285/290), and http-request inputs belong on the start node (ESQ1-286). Deliberately does NOT restate the node whitelist: the generator's shared system prompt already carries it, and duplicating it would burn planner output budget that ESQ1-300 already implicates. Co-Authored-By: Claude Opus 5 (1M context) --- api/services/dify_builder/agent/build.py | 18 ++++++ .../dify_builder/test_build_cognition.py | 60 +++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/api/services/dify_builder/agent/build.py b/api/services/dify_builder/agent/build.py index 3e5f306f29b..818e234dec8 100644 --- a/api/services/dify_builder/agent/build.py +++ b/api/services/dify_builder/agent/build.py @@ -348,6 +348,24 @@ _WORKFLOW_TOPOLOGY_DIRECTIVE = ( "Build a Dify WORKFLOW graph (this is NOT a chat app): it MUST begin with exactly one " "'start' node and terminate in at least one 'end' node that returns the result. Do NOT use " "'answer' nodes -- those exist only in chat / advanced-chat apps and are invalid in a workflow." + "\n\n" + # Node-usage guidance (meeting item 9). Builder-only: this string is + # prepended to the instruction at build.py:377 and cmd+K never sees it. + # Only rules that earned their place by breaking a real build belong here -- + # every line costs planner output budget, which ESQ1-300 implicates. + "Node usage rules:\n" + "- A node may only reference variables its upstream node actually declares. A 'tool' node " + "exposes 'text', 'files' and 'json' (plus whatever its provider declares) -- it does NOT " + "expose 'result'. A 'code' node exposes exactly the keys in its own outputs map.\n" + "- 'document-extractor' requires a 'file' or 'file-list' input variable on the start node; " + "feed its 'text' output into the node that consumes it.\n" + "- Children of an 'iteration' or 'loop' belong INSIDE the container via their parent " + "reference. Do not wire a container's body as sibling top-level nodes.\n" + "- Compare like with like in 'if-else': a numeric variable needs a numeric operator, a " + "string variable a string operator.\n" + "- Use a 'tool' node for a third-party service only when the plan names an installed tool; " + "otherwise use 'http-request' and expose its changing inputs as start-node variables rather " + "than hardcoding them." ) diff --git a/api/tests/unit_tests/services/dify_builder/test_build_cognition.py b/api/tests/unit_tests/services/dify_builder/test_build_cognition.py index 05a9a60fa65..e3a42f211ee 100644 --- a/api/tests/unit_tests/services/dify_builder/test_build_cognition.py +++ b/api/tests/unit_tests/services/dify_builder/test_build_cognition.py @@ -318,3 +318,63 @@ def test_plan_prompt_still_requests_json_and_language(monkeypatch): assert '{"plan": ["step", ...]}' in captured["system"] assert build.llm.json_language_instruction("plan steps") in captured["system"] + + +def test_topology_directive_keeps_the_workflow_rules(): + """Regression guard: the original topology contract must survive. This + directive is why workflow mode never gets 'answer' nodes.""" + d = build._WORKFLOW_TOPOLOGY_DIRECTIVE + assert "'start'" in d + assert "'end'" in d + assert "answer" in d + + +def test_topology_directive_carries_node_usage_guidance(): + """Meeting item 9, tier 2: usage detail rides the Builder-only prepend.""" + d = build._WORKFLOW_TOPOLOGY_DIRECTIVE + lowered = d.lower() + # the config mistakes that actually broke real builds + assert "document-extractor" in lowered # needs a file input + assert "iteration" in lowered # children belong inside the container + assert "variable" in lowered # reference only declared outputs + + +def test_topology_directive_does_not_restate_the_whitelist(): + """The generator's SHARED system prompt already lists every node type. + Restating it here burns planner output budget for nothing (see ESQ1-300).""" + assert build._DIFY_NODE_VOCABULARY not in build._WORKFLOW_TOPOLOGY_DIRECTIVE + + +def test_build_nodes_sends_the_directive_to_the_generator(monkeypatch): + """The directive is only worth anything if it actually reaches the + generator ahead of the plan items.""" + seen = {} + + class _FakeGen: + @staticmethod + def generate_workflow_graph( + *, + tenant_id, + mode, + instruction, + model_config, + current_graph, # noqa: ARG004 + ): + seen["instruction"] = instruction + return { + "graph": { + "nodes": [{"id": "node1", "data": {"type": "start", "title": "Start"}}], + "edges": [], + } + } + + # Patch the module-level reference (build.py:21 imports the class), NOT the + # shared class object itself. + monkeypatch.setattr(build, "WorkflowGeneratorService", _FakeGen) + monkeypatch.setattr(build, "_generator_model_config", lambda *a, **k: {}) # noqa: ARG005 + + build.build_nodes(tenant_id="t1", model_config={}, plan_items=["llm node drafts a reply"]) + + instruction = seen["instruction"] + assert instruction.startswith(build._WORKFLOW_TOPOLOGY_DIRECTIVE) + assert "llm node drafts a reply" in instruction