Commit Graph
3 Commits
Author SHA1 Message Date
Charles YaoandClaude Opus 5 c4ac3d49bf fix(dify-builder): a proposal is engine-checked before it reaches the gate
`filter_applicable` dry-ran STRUCTURE only. In the live F4 Edit four intents
came back "applicable, 0 rejected", the batch then died at `apply_repair`'s
preflight, and the one corrective re-prompt could never fire -- it is keyed on
`rejected` being non-empty. Every re-approval re-sent a byte-identical prompt
and got a byte-identical batch back.

`filter_applicable` now returns its dry-run graph (`DryRun` NamedTuple) after
running it through the shared `heal_nodes_for_preflight` -- the same
normalizers, at the same point in the sequence, the write chokepoint runs. A
dry run that healed less would report a defect the write was going to fix; one
that healed more would pass a batch the write refuses. It also skips the
intents the port drops as already-present, via a predicate both now share, so
a re-sent duplicate `create_node` no longer burns the re-prompt.

`preflight.vet_intents` then puts that graph through `validate_node_config` and
feeds any NEW problem into the existing re-prompt, quoting the engine's own
message. `new_preflight_problems` is the one definition of that rule, shared
with `apply_repair` so the dry run and the write cannot answer differently.

It is keyed on the set of pydantic error LOCATIONS per node, which is the only
key that gets both halves right. Node identity exempts an already-refused node
from every later verdict -- and since a Fix repair targets a node that is by
definition already invalid, that would leave `propose_repair` with no node-data
coverage on its own culprit. Message text is too fine: pydantic quotes a
truncated repr of the input, so filling one of three missing fields rewrites
the other two messages and an exact-text diff refuses the repair that improved
the node.

The refusal text is rebuilt from a credential-redacted copy of the node. The
verdict still comes from the real one; only the words the model is shown are
withheld, because a model-level pydantic error echoes the node's whole `data`
and that is where a live bearer token sits.

The retry budget is unchanged: one corrective re-prompt, then Edit's existing
fallback to the first attempt's applicable intents and Fix's `_no_fix()`.

Also folds in the Task 1 review: the generator's postprocess calls the shared
heal set plus `derive_if_else_var_types` instead of re-spelling three
normalizers inline, and the valid-operator no-op sweep covers the
list-operator container so `FilterOperator` gets its own check.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-22 19:23:12 -07:00
Charles YaoandClaude Opus 5 71cb5688ac fix(dify-builder): a default may fill a blank, never invent a purpose
The question-classifier default shipped two `classes` with blank names. That
turns a loud preflight refusal into a node that is written, reported as a
success, and silently classifies into two meaningless categories -- and the
fabricated ids `["1", "2"]` pin `declared_branch_handles`, so a semantic
`connect` is dropped while the create survives. That is the failure shape the
registry exists to eliminate.

Draw the line explicitly and apply it to every entry: a default may supply a
field whose empty value is semantically NEUTRAL -- a container, a mode, a config
or auth block -- but never one that carries the node's PURPOSE, where an empty
value produces a node that RUNS and yields nothing. Dropped, with the engine
behaviour each empty value would have produced:

  end.outputs                  End runs OK, so run_finished_without_output is
                               False -> "All checks passed" on an empty result
  answer.answer                streams nothing
  template-transform.template  renders ""
  variable-aggregator.variables  SUCCEEDED, outputs={} -- triage cause (d)
  question-classifier.classes  two blank categories + fabricated handle ids
  question-classifier.query_variable_selector  classifies `query or ""`
  knowledge-retrieval.dataset_ids  retrieves from no dataset, succeeds empty

The whole `if-else` entry goes too, though nothing is gained or lost by it: its
one case has an empty `conditions` list that graphon combines with `all([])`, so
it always matches -- but `iter_cases()` synthesizes that identical case when
`cases` is absent, so dropping the entry does not cure it. It only stops us
writing a fabricated always-true condition that makes the node look configured.

An empty value that fails LOUDLY is not in that category: there is no silence to
protect against, and dropping it would pay the full batch-refusal cost for
nothing. Verified loud, so these stay -- `llm.model` (never launches, "model not
configured"), `llm.prompt_template` (empty messages are filtered out, leaving
zero, so graphon raises `NoPromptFoundError`), `code.code` (the sandbox runner
calls `main(**inputs_obj)` with no `main` defined), `code.outputs` (graphon
raises `CodeNodeError("Not all output parameters are validated.")` for any key
the code returns), and `http-request.url` (`Executor._init_url` raises
`InvalidURLError("url is required")` before any request goes out).
`template-transform.variables` stays too -- it is structural, and it is the
exact field the live F4 failure was missing.

The placeholder agent genuinely means "no dataset, no output", so it now says
so itself instead of borrowing a default that would say it for every caller.

The engine sweep is re-pinned in both directions: the fields the engine reports
missing from a node built from a default ALONE must be exactly the purpose
fields the registry refuses to fabricate -- no more (a structural regression)
and no fewer (a silent-success regression).

Also: name the `filter_applicable` test after what it actually asserts, since
that function does not return its working graph; and reword the comment in
`graph_normalizers` that quoted a bare `type` marker, one reflow away from
being read as a type comment and silently killing the type check again.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-22 17:28:05 -07:00
Charles YaoandClaude Opus 5 9f766e876a fix(dify-builder): a created node carries the fields its type requires
An Edit LLM writes only the fields it was thinking about. graphon requires a
handful per node type with no default, so `create_node{template-transform,
{title, template}}` produced a node the draft preflight refused -- and the
refusal killed the WHOLE repair batch, parking the session at
`edit.plan_approval` with an identical card on every approval (triage
`edit-branch-failure-2026-09-22` cause (b)).

Extend the node-type default registry to every type the Build/Edit/Fix agents
may create -- template-transform, code, http-request, variable-aggregator,
answer, question-classifier, if-else, tool -- each taken from the matching
`web/app/components/workflow/nodes/<type>/default.ts` and each cited alongside
the graphon field it covers. Add `default_config_or_empty`, a non-raising
lookup: a type the registry does not cover contributes no defaults rather than
aborting the edit that is creating it.

Merge those defaults UNDER the caller's config at `graph_ops._build_node`, the
one construction point every created node passes through. A field the caller
supplied always wins; only a missing one is filled. That covers Build, Edit and
Fix, `create_node` and `insert_between`, and the `filter_applicable` dry run,
so the dry run sees exactly the node the port will write.

Defaults fill MISSING required fields; a field that is present but WRONG stays
`graph_normalizers.heal_nodes_for_preflight`'s job.

Two editor-only keys are deliberately not copied. `_targetBranches` (if-else,
question-classifier) is frontend state the canvas regenerates from
`cases`/`classes`, which is also where `declared_branch_handles` reads handles
from. A caller-supplied `cases`/`classes` wins the merge while a default
`_targetBranches` would not, so shipping it would attach a stale branch list to
every Builder-created branch node.

Move the registry from `services/dify_builder/` to `core/dify_builder/`: it was
imported by `core/dify_builder/placeholder_agent.py`, against the constraint
that `core/` must not import from `services/`. The dependency now points the
legal way, and the module stays pure -- copy, typing and graphon only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-22 16:52:09 -07:00