diff --git a/api/services/dify_builder/agent/build.py b/api/services/dify_builder/agent/build.py index a0e01573174..8cbc1a83ba6 100644 --- a/api/services/dify_builder/agent/build.py +++ b/api/services/dify_builder/agent/build.py @@ -169,14 +169,18 @@ def _ground(intents: list[MutationIntent], mc: ModelConfig, tenant_id: str, plan if intent.op != "create_node": continue config = intent.args.get("config") or {} - if intent.args.get("node_type") == "llm": + if isinstance(config.get("model"), dict): # Ground ONLY provider+name (the real, configured model); preserve whatever # mode/completion_params the generator produced -- never fabricate params. - model = dict(config.get("model") or {}) + # Applies to ANY node type that carries a model block (llm, + # question-classifier, parameter-extractor, ...), not just "llm" -- + # otherwise a drifted/hallucinated model on those node types flows + # through ungrounded. + model = dict(config["model"]) model["provider"] = mc.provider model["name"] = mc.name model.setdefault("mode", mode) config["model"] = model - elif intent.args.get("node_type") == "knowledge-retrieval": + if intent.args.get("node_type") == "knowledge-retrieval": config["dataset_ids"] = list(matched) # independent copy per node -- never share one list intent.args["config"] = config diff --git a/api/services/dify_builder/agent/graph_translate.py b/api/services/dify_builder/agent/graph_translate.py index 5c0a4602640..f5384cad2dd 100644 --- a/api/services/dify_builder/agent/graph_translate.py +++ b/api/services/dify_builder/agent/graph_translate.py @@ -45,9 +45,16 @@ def to_intents(graph: dict[str, Any]) -> list[MutationIntent]: src, tgt = edge.get("source"), edge.get("target") if not src or not tgt: continue - connects.append(MutationIntent(op="connect", args={ + args: dict[str, Any] = { "from_node": remap.get(str(src), str(src)), "to_node": remap.get(str(tgt), str(tgt)), - })) + } + source_handle = edge.get("sourceHandle") + if source_handle: + args["source_handle"] = source_handle + target_handle = edge.get("targetHandle") + if target_handle: + args["target_handle"] = target_handle + connects.append(MutationIntent(op="connect", args=args)) return creates + connects diff --git a/api/tests/unit_tests/core/dify_builder/test_handlers_build.py b/api/tests/unit_tests/core/dify_builder/test_handlers_build.py index f9ba9663622..38f94d0967c 100644 --- a/api/tests/unit_tests/core/dify_builder/test_handlers_build.py +++ b/api/tests/unit_tests/core/dify_builder/test_handlers_build.py @@ -68,7 +68,8 @@ def test_build_form_fields_whitelists_and_coerces_type(): ] ) assert [f.key for f in fields] == ["a", "b"] - assert fields[0].type == "select" and fields[0].options == ["x"] + assert fields[0].type == "select" + assert fields[0].options == ["x"] assert fields[1].type == "text" # unknown type coerced to text @@ -416,7 +417,8 @@ def test_governance_automatic_learns_and_reaches_complete(): res = handle_governance_feedback(env, Turn(actor=_actor()), *repo.get_session(s.id)) assert res.next == PcState.BUILD_COMPLETE kinds = [i.kind for i in res.items] - assert "build_learning" in kinds and "summary" in kinds + assert "build_learning" in kinds + assert "summary" in kinds assert "notice" in kinds assert env.agent.learn_calls == 1 # seam called for automatic @@ -454,7 +456,8 @@ def test_governance_ask_rests_at_await_learning_with_pending_card(): res = handle_governance_feedback(env, Turn(actor=_actor()), *repo.get_session(s.id)) assert res.next == PcState.BUILD_AWAIT_LEARNING bl = [i for i in res.items if i.kind == "build_learning"][0] - assert bl.payload["policy"] == "ask" and bl.payload["state"] == "pending" + assert bl.payload["policy"] == "ask" + assert bl.payload["state"] == "pending" assert getattr(env.agent, "learn_calls", 0) == 0 # not learned until accepted @@ -474,7 +477,8 @@ def test_await_learning_accept_learns_and_completes(): ) assert res.next == PcState.BUILD_COMPLETE kinds = [i.kind for i in res.items] - assert "decision" in kinds and "summary" in kinds + assert "decision" in kinds + assert "summary" in kinds assert "notice" in kinds assert env.agent.learn_calls == 1 diff --git a/api/tests/unit_tests/core/dify_builder/test_placeholder_agent.py b/api/tests/unit_tests/core/dify_builder/test_placeholder_agent.py index cdc7baadbf7..ded72deba20 100644 --- a/api/tests/unit_tests/core/dify_builder/test_placeholder_agent.py +++ b/api/tests/unit_tests/core/dify_builder/test_placeholder_agent.py @@ -195,4 +195,5 @@ def test_placeholder_learn_from_build_returns_descriptor(): from core.dify_builder.placeholder_agent import PlaceholderAgent out = PlaceholderAgent().learn_from_build("summarize a PDF", {}, ["step"], ["llm"]) - assert isinstance(out, str) and out != "" + assert isinstance(out, str) + assert out != "" diff --git a/api/tests/unit_tests/services/dify_builder/test_build_nodes.py b/api/tests/unit_tests/services/dify_builder/test_build_nodes.py index a02e075af16..9d79fefd9fa 100644 --- a/api/tests/unit_tests/services/dify_builder/test_build_nodes.py +++ b/api/tests/unit_tests/services/dify_builder/test_build_nodes.py @@ -73,6 +73,51 @@ def test_build_nodes_degrades_to_empty_on_generator_error(): assert build.build_nodes("t1", {}, ["x"]) == [] +def test_build_nodes_grounds_model_on_question_classifier_node(): + gen_graph_qc = { + "graph": { + "nodes": [ + {"id": "s", "type": "custom", "data": {"type": "start", "title": "Start", "variables": []}}, + { + "id": "qc1", + "type": "custom", + "data": { + "type": "question-classifier", + "title": "Classify", + "model": {"provider": "wrong", "name": "wrong"}, + "classes": [], + }, + }, + {"id": "e", "type": "custom", "data": {"type": "end", "title": "End", "outputs": []}}, + ], + "edges": [ + {"id": "e1", "source": "s", "target": "qc1"}, + {"id": "e2", "source": "qc1", "target": "e"}, + ], + }, + "error": "", + "errors": [], + } + with ( + patch.object(build, "_generator_model_config", return_value=_fake_mc("anthropic", "claude-opus-4-8")), + patch( + "services.dify_builder.agent.build.WorkflowGeneratorService.generate_workflow_graph", + return_value=gen_graph_qc, + ), + ): + intents = build.build_nodes("t1", {}, ["Classify the request"]) + + by_type = {} + for intent in intents: + if intent.op == "create_node": + by_type[intent.args["node_type"]] = intent.args["config"] + assert by_type["question-classifier"]["model"]["provider"] == "anthropic" # ungrounded model overwritten + assert by_type["question-classifier"]["model"]["name"] == "claude-opus-4-8" + # no fabricated params: the fixture's node carried no completion_params, and none + # should be synthesized (e.g. a guessed temperature) during grounding. + assert "completion_params" not in by_type["question-classifier"]["model"] + + def test_build_nodes_dataset_ids_are_independent_lists_per_node(): gen_graph_two_kb = { "graph": { diff --git a/api/tests/unit_tests/services/dify_builder/test_graph_translate.py b/api/tests/unit_tests/services/dify_builder/test_graph_translate.py index 3b677cac1f0..e0d1a0c3593 100644 --- a/api/tests/unit_tests/services/dify_builder/test_graph_translate.py +++ b/api/tests/unit_tests/services/dify_builder/test_graph_translate.py @@ -15,14 +15,17 @@ def test_to_intents_translates_nodes_then_edges_and_normalizes_start(): assert ops == [("create_node", "start"), ("create_node", "n_llm"), ("connect", ("start", "n_llm"))] # config carries data minus the type key; create_node args expose node_type start = intents[0] - assert start.args["node_type"] == "start" and "type" not in start.args["config"] + assert start.args["node_type"] == "start" + assert "type" not in start.args["config"] assert start.args["config"]["title"] == "Start" + def test_to_intents_skips_malformed_nodes(): from services.dify_builder.agent.graph_translate import to_intents graph = {"nodes": [{"id": "x", "data": {}}, {"id": "", "data": {"type": "llm"}}], "edges": []} assert to_intents(graph) == [] # no type / no id -> dropped + def test_to_intents_multiple_start_nodes_only_first_normalized(): from services.dify_builder.agent.graph_translate import to_intents # Graph with two start-type nodes: only first should be renamed to "start", second keeps original id @@ -59,3 +62,36 @@ def test_to_intents_multiple_start_nodes_only_first_normalized(): assert connects[0].args["from_node"] == "start", "Edge from s1 should be remapped to 'start'" # Second edge should connect from "s2" (not remapped) to "n_llm" assert connects[1].args["from_node"] == "s2", "Edge from s2 should keep 's2'" + + +def test_to_intents_carries_branch_handles_through_connect(): + from services.dify_builder.agent.graph_translate import to_intents + + graph = { + "nodes": [ + {"id": "if1", "type": "custom", "data": {"type": "if-else", "title": "If"}}, + {"id": "n_true", "type": "custom", "data": {"type": "llm", "title": "True branch"}}, + ], + "edges": [ + {"id": "e1", "source": "if1", "target": "n_true", "sourceHandle": "true", "targetHandle": "target"}, + ], + } + intents = to_intents(graph) + connect = next(i for i in intents if i.op == "connect") + assert connect.args["source_handle"] == "true" + assert connect.args["target_handle"] == "target" + + +def test_to_intents_connect_without_handles_has_no_handle_keys(): + from services.dify_builder.agent.graph_translate import to_intents + + graph = { + "nodes": [ + {"id": "n1", "type": "custom", "data": {"type": "start", "title": "Start"}}, + {"id": "n2", "type": "custom", "data": {"type": "llm", "title": "LLM"}}, + ], + "edges": [{"id": "e1", "source": "n1", "target": "n2"}], + } + intents = to_intents(graph) + connect = next(i for i in intents if i.op == "connect") + assert connect.args == {"from_node": "start", "to_node": "n2"} diff --git a/api/tests/unit_tests/services/dify_builder/test_resources.py b/api/tests/unit_tests/services/dify_builder/test_resources.py index 8c2f83c2d06..bdb13009988 100644 --- a/api/tests/unit_tests/services/dify_builder/test_resources.py +++ b/api/tests/unit_tests/services/dify_builder/test_resources.py @@ -6,19 +6,27 @@ def test_list_tenant_resources_normalizes_all_three_kinds(): class _Model: def __init__(self, m): self.model = m + class _ProvResp: def __init__(self, p, ms): self.provider, self.models = p, [_Model(m) for m in ms] + class _DS: def __init__(self, i, n): self.id, self.name = i, n + tool_record = { + "provider_name": "google", "tool_name": "search", + "tool_label": "Google Search", "description": "", + } with patch.object(resources, "_list_models", return_value=[_ProvResp("anthropic", ["claude-opus-4-8"])]), \ patch.object(resources, "_list_datasets", return_value=[_DS("kb-1", "Company KB")]), \ - patch.object(resources, "_list_tools", return_value=[{"provider_name": "google", "tool_name": "search", "tool_label": "Google Search", "description": ""}]): + patch.object(resources, "_list_tools", return_value=[tool_record]): out = resources.list_tenant_resources("t1") assert out.models[0].id == "anthropic/claude-opus-4-8" - assert out.datasets[0].id == "kb-1" and out.datasets[0].label == "Company KB" - assert out.tools[0].id == "google/search" and out.tools[0].label == "Google Search" + assert out.datasets[0].id == "kb-1" + assert out.datasets[0].label == "Company KB" + assert out.tools[0].id == "google/search" + assert out.tools[0].label == "Google Search" def test_each_source_degrades_to_empty(): @@ -36,14 +44,20 @@ def test_mixed_failure_partial_success(): class _DS: def __init__(self, i, n): self.id, self.name = i, n + tool_record = { + "provider_name": "google", "tool_name": "search", + "tool_label": "Google Search", "description": "", + } with patch.object(resources, "_list_models", side_effect=RuntimeError("boom")), \ patch.object(resources, "_list_datasets", return_value=[_DS("kb-1", "Company KB")]), \ - patch.object(resources, "_list_tools", return_value=[{"provider_name": "google", "tool_name": "search", "tool_label": "Google Search", "description": ""}]): + patch.object(resources, "_list_tools", return_value=[tool_record]): out = resources.list_tenant_resources("t1") assert out.models == [] - assert len(out.datasets) == 1 and out.datasets[0].id == "kb-1" - assert len(out.tools) == 1 and out.tools[0].id == "google/search" + assert len(out.datasets) == 1 + assert out.datasets[0].id == "kb-1" + assert len(out.tools) == 1 + assert out.tools[0].id == "google/search" def test_malformed_tool_record_degrades(): @@ -55,4 +69,5 @@ def test_malformed_tool_record_degrades(): out = resources.list_tenant_resources("t1") assert out.tools == [] - assert out.models == [] and out.datasets == [] + assert out.models == [] + assert out.datasets == []