From f12d8f4e0f88fe1c54ebd7dc26ec9dc53d922a78 Mon Sep 17 00:00:00 2001 From: Charles Yao Date: Sun, 20 Sep 2026 23:05:34 -0700 Subject: [PATCH] feat(dify-builder): plan in Dify node vocabulary, not business language propose_plan_v1's system prompt was two sentences with no node vocabulary, so it planned in business language -- "Send a Feishu notification", "Connect to the database". Those steps became the generator's "# User instruction" verbatim, asking it to map work onto nodes that do not exist, and the failure only surfaced at build-generate-graph, minutes after the user had approved the plan (ESQ1-279). The generator already knows the node types: its SHARED system prompt carries the whitelist. The gap was upstream, in the Builder's own planner. Naming the vocabulary here makes that existing whitelist sufficient. Builder-only: nothing under core/workflow/generator/ is touched, so the cmd+K /create and /refine features are unaffected. Co-Authored-By: Claude Opus 5 (1M context) --- api/services/dify_builder/agent/build.py | 48 +++++++- .../dify_builder/test_build_cognition.py | 104 ++++++++++++++++-- 2 files changed, 136 insertions(+), 16 deletions(-) diff --git a/api/services/dify_builder/agent/build.py b/api/services/dify_builder/agent/build.py index 42a098b887e..3e5f306f29b 100644 --- a/api/services/dify_builder/agent/build.py +++ b/api/services/dify_builder/agent/build.py @@ -97,6 +97,37 @@ def _degraded_form(goal_text: str) -> dict[str, Any]: } +# The node types a plan step may map to. This exists because the Builder's own +# planner previously had NO node vocabulary (ESQ1-279): it planned in business +# language ("Send a Feishu notification"), and the generator -- whose system +# prompt DOES carry a node whitelist -- was then handed steps it could not map +# onto any real node, failing only after the user had approved the plan. +# +# Deliberately Builder-only. The generator's own whitelist lives in the SHARED +# core/workflow/generator/prompts/planner_prompts.py, which cmd+K also uses and +# which this package must not touch. Keep the two conceptually aligned, but do +# not try to import across that boundary. +_DIFY_NODE_VOCABULARY = """\ +- "start" — workflow entry. Always present. Holds the input form variables. +- "end" — workflow exit. Returns the result. +- "llm" — call an LLM with a prompt. +- "knowledge-retrieval" — query a Dify knowledge base. +- "code" — run a Python/JavaScript snippet. +- "template-transform" — Jinja2 string templating. +- "http-request" — call an external HTTP API. +- "tool" — call a Dify built-in or installed plugin tool (web search, Slack, JSON, …). +- "if-else" — branch on a condition. +- "iteration" — run a sub-pipeline over every item of a list. +- "loop" — repeat a sub-pipeline until an exit condition holds. +- "question-classifier" — route to a labelled branch by free-text intent. +- "parameter-extractor" — pull structured fields out of free text with an LLM. +- "document-extractor" — extract text from an uploaded file. Needs a file input. +- "variable-aggregator" — rejoin mutually-exclusive branches into one variable. +- "list-operator" — filter / sort / slice an array variable. +- "assigner" — update an existing conversation or loop variable. +- "human-input" — pause for a person to review, approve, or enter data.""" + + def propose_plan_v1( model, requirements: dict[str, Any], @@ -106,7 +137,18 @@ def propose_plan_v1( return _degraded_plan() system = ( "You are a Dify workflow planner. Given requirements, propose an ordered list of " - 'concise build steps. Reply with ONLY JSON: {"plan": ["step", ...]}.' + "concise build steps.\n\n" + "Every step MUST be implementable by exactly one of these Dify node types, and you " + "may plan ONLY in these terms:\n\n" + f"{_DIFY_NODE_VOCABULARY}\n\n" + "Write each step as the work that node does, naming the node type. Good: " + '"llm node drafts a reply from the ticket text". Bad: "Notify the team on Feishu" ' + "-- that names a product, not a node.\n\n" + "Dify has no scheduler, database, email or messaging node. A step that needs one of " + "those is either a `tool` node (when an installed plugin provides it), an " + "`http-request` node, or outside the workflow entirely -- say so in the step rather " + "than inventing a node.\n\n" + 'Reply with ONLY JSON: {"plan": ["step", ...]}.' ) + llm.json_language_instruction("plan steps") try: data = llm.invoke_json( @@ -294,9 +336,7 @@ def _selected_workflow_model(tenant_id: str, resource_ids: Sequence[str]) -> Mod provider, _, name = selected.rpartition("/") if not provider or not name: return None - return ModelConfig.model_validate( - {"provider": provider, "name": name, "mode": "chat", "completion_params": {}} - ) + return ModelConfig.model_validate({"provider": provider, "name": name, "mode": "chat", "completion_params": {}}) # Prepended to the generator instruction so the LLM emits a WORKFLOW-shaped graph 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 dd8828bc7e4..05a9a60fa65 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 @@ -30,20 +30,32 @@ class _BoomInstance: def test_analyze_goal_returns_fields_and_values(): - m = _FakeInstance([json.dumps({ - "fields": [{"key": "categories", "label": "Categories", "type": "text"}], - "values": {"categories": "billing"}, - })]) + m = _FakeInstance( + [ + json.dumps( + { + "fields": [{"key": "categories", "label": "Categories", "type": "text"}], + "values": {"categories": "billing"}, + } + ) + ] + ) out = build.analyze_goal(m, "triage tickets") assert out["fields"][0]["key"] == "categories" assert out["values"] == {"categories": "billing"} def test_analyze_goal_reconciles_field_type_with_value(): - m = _FakeInstance([json.dumps({ - "fields": [{"key": "max_results", "label": "Max results", "type": "text"}], - "values": {"max_results": 5}, - })]) + m = _FakeInstance( + [ + json.dumps( + { + "fields": [{"key": "max_results", "label": "Max results", "type": "text"}], + "values": {"max_results": 5}, + } + ) + ] + ) out = build.analyze_goal(m, "limit search results") @@ -83,8 +95,13 @@ def test_propose_plan_v1_degrades_on_boom(): def test_discover_resources_grounds_real_ids(monkeypatch): - monkeypatch.setattr(build.resources, "list_tenant_resources", lambda t: resources.TenantResources( # noqa: ARG005 - models=[], datasets=[resources.ResourceRef(id="kb-1", label="Company KB")], tools=[])) + monkeypatch.setattr( + build.resources, + "list_tenant_resources", + lambda t: resources.TenantResources( # noqa: ARG005 + models=[], datasets=[resources.ResourceRef(id="kb-1", label="Company KB")], tools=[] + ), + ) m = _FakeInstance([json.dumps({"resource_ids": ["kb-1"]})]) opts = build.discover_resources(m, "t1", ["Retrieve knowledge"]) assert opts[0].id == "kb-1" @@ -223,8 +240,13 @@ def test_assess_capability_gap_shows_the_model_each_option_and_its_readiness(mon def test_bind_resources_names_bound_label(monkeypatch): - monkeypatch.setattr(build.resources, "list_tenant_resources", lambda t: resources.TenantResources( # noqa: ARG005 - models=[], datasets=[resources.ResourceRef(id="kb-1", label="Company KB")], tools=[])) + monkeypatch.setattr( + build.resources, + "list_tenant_resources", + lambda t: resources.TenantResources( # noqa: ARG005 + models=[], datasets=[resources.ResourceRef(id="kb-1", label="Company KB")], tools=[] + ), + ) out = build.bind_resources(None, "t1", ["Retrieve knowledge"], ["kb-1"]) assert any("Company KB" in item for item in out) @@ -238,3 +260,61 @@ def test_learn_from_build_degrades_on_boom(): out = build.learn_from_build(m, "some goal", {}, ["plan"], ["node1"]) assert isinstance(out, str) assert "1-node workflow" in out + + +def test_plan_prompt_carries_the_node_vocabulary(monkeypatch): + """ESQ1-279: the Builder's own planner had NO node vocabulary, so it + planned in business language ("Send a Feishu notification") and the + generator was then asked to map steps onto nodes that do not exist.""" + captured = {} + + def fake_invoke_json(model, *, system, user, **kw): # noqa: ARG001 + captured["system"] = system + captured["user"] = user + return {"plan": ["Start node collects the review list"]} + + monkeypatch.setattr(build.llm, "invoke_json", fake_invoke_json) + build.propose_plan_v1(_FakeInstance([]), {"goal": "summarise reviews"}) + + system = captured["system"] + # the vocabulary is present, and is the shared constant + assert build._DIFY_NODE_VOCABULARY in system + # a representative spread of node types the planner may use + for node_type in ("start", "end", "llm", "iteration", "tool", "if-else", "document-extractor"): + assert f'"{node_type}"' in system, f"missing node type {node_type}" + # and it is told to plan ONLY in these terms + assert "only" in system.lower() + + +def test_plan_prompt_names_capabilities_dify_lacks(monkeypatch): + """The failure mode is inventing steps Dify has no node for. Name the + common ones explicitly so the model routes them instead of inventing.""" + captured = {} + + def fake_invoke_json(model, *, system, user, **kw): # noqa: ARG001 + captured["system"] = system + return {"plan": ["step"]} + + monkeypatch.setattr(build.llm, "invoke_json", fake_invoke_json) + build.propose_plan_v1(_FakeInstance([]), {}) + + lowered = captured["system"].lower() + assert "schedul" in lowered # no timer/trigger node exists + assert "tool" in lowered # third-party sends go through a tool node + + +def test_plan_prompt_still_requests_json_and_language(monkeypatch): + """Regression guard: the JSON contract and the reply-language instruction + must survive the rewrite -- invoke_json parses {"plan": [...]} and the + Builder replies in the user's language.""" + captured = {} + + def fake_invoke_json(model, *, system, user, **kw): # noqa: ARG001 + captured["system"] = system + return {"plan": ["step"]} + + monkeypatch.setattr(build.llm, "invoke_json", fake_invoke_json) + build.propose_plan_v1(_FakeInstance([]), {}) + + assert '{"plan": ["step", ...]}' in captured["system"] + assert build.llm.json_language_instruction("plan steps") in captured["system"]