From b9ee13f0a95ecf733d3d2791fcdc5ff413a9b356 Mon Sep 17 00:00:00 2001 From: Charles Yao Date: Mon, 21 Sep 2026 22:25:00 -0700 Subject: [PATCH] fix(dify-builder): give Edit the repair breaker Build has Edit's affected-path test diagnosed and staged a repair for the same engine failure with no counter at all, so the loop 6877532f8e closed for Build stayed open for Edit. The breaker (engine-keyed signature, consecutive-repeat counter, MAX_REPEATED_REPAIRS) moves to handlers_fix under public names, Build keeps its underscore names as aliases, and Edit stops at its gate on the third identical failure. A new edit goal resets the counter. Co-Authored-By: Claude Opus 5 (1M context) --- api/core/dify_builder/handlers_build.py | 65 +++---------------- api/core/dify_builder/handlers_edit.py | 34 ++++++++++ api/core/dify_builder/handlers_fix.py | 61 +++++++++++++++++ .../core/dify_builder/test_handlers_edit.py | 41 ++++++++++++ 4 files changed, 146 insertions(+), 55 deletions(-) diff --git a/api/core/dify_builder/handlers_build.py b/api/core/dify_builder/handlers_build.py index 6bb130c39d9..4452f5e972c 100644 --- a/api/core/dify_builder/handlers_build.py +++ b/api/core/dify_builder/handlers_build.py @@ -35,6 +35,7 @@ from core.dify_builder.contract import ( TestStat, ) from core.dify_builder.handlers_fix import ( + MAX_REPEATED_REPAIRS, NO_OUTPUT_BODY, NO_OUTPUT_REPLY, UNKNOWN_OUTCOME_STUCK_BODY, @@ -47,14 +48,17 @@ from core.dify_builder.handlers_fix import ( build_form_fields, dead_end_branch_node_id, emit_canvas, + failure_signature, first_failed_node, is_input_failure, launch_error_text, merge_known_keys, mint_checkpoint, model_config_error_text, + note_repair_error, note_unknown_outcome, perform_revert, + repair_is_repeating, run_finished_without_output, start_schema, testdata_form_fields, @@ -1165,61 +1169,12 @@ def handle_test_and_repair(env: Env, turn: Turn, s: Session, fc: DifyBuilderCont ) -_MAX_REPEATED_REPAIRS = 2 - - -def _failure_signature(run: Run) -> str: - """A STABLE key for "this is the same failure again", built from the - ENGINE's own output: the first failed node's id plus its error text, or the - launch error when the run threw before any node ran. Whitespace is - collapsed so formatting alone cannot look like a new failure. Returns "" - when the run carries nothing identifying -- an unknown failure must never - compare equal to another unknown one. - - Deliberately NOT ``diagnosis.root_cause``. That is LLM prose, regenerated - on every diagnosis call: in one observed session it was reworded on each of - four turns and switched to Chinese on one of them. Keying on it meant - ``error == fc.last_repair_error`` could never hold, so ``repair_attempts`` - reset forever and ``_repair_is_repeating`` was unreachable -- that session - burned 9 failed runs and 8 repair approvals with this guard in place. - """ - for node in run.per_node: - if node.status == "failed": - node_error = " ".join((node.error or "").split()) - if node.node_id or node_error: - return f"{node.node_id}|{node_error}" - launch_error = " ".join((run.error or "").split()) - return f"|{launch_error}" if launch_error else "" - - -def _note_repair_error(fc: DifyBuilderContext, run: Run) -> None: - """Record this failure's signature and count CONSECUTIVE repeats of it. - - ``repair_attempts`` is not a global budget: a different failure means the - loop is still making progress, however slowly, so the count restarts. - The same failure again means the repair that just ran did not address the - cause, so the count advances. Call this once per diagnosis, before - ``_repair_is_repeating``. - - Takes the ``Run`` rather than a message so the key comes from the engine - (see ``_failure_signature``) and not from anything an LLM wrote. - """ - signature = _failure_signature(run) - if signature and signature == fc.last_repair_error: - fc.repair_attempts += 1 - else: - fc.repair_attempts = 0 - fc.last_repair_error = signature - - -def _repair_is_repeating(fc: DifyBuilderContext) -> bool: - """Has the SAME error now survived ``_MAX_REPEATED_REPAIRS`` repairs? - - Reads the counter ``_note_repair_error`` maintains; the identical error - that many times over means the repair agent is aiming at something that - isn't the cause, and another round will not find it. - """ - return fc.repair_attempts >= _MAX_REPEATED_REPAIRS +# The repair breaker is shared with Edit and owned by handlers_fix; these +# names stay for the existing call sites and tests in this module. +_MAX_REPEATED_REPAIRS = MAX_REPEATED_REPAIRS +_failure_signature = failure_signature +_note_repair_error = note_repair_error +_repair_is_repeating = repair_is_repeating def handle_await_repair(env: Env, turn: Turn, s: Session, fc: DifyBuilderContext) -> StepResult: diff --git a/api/core/dify_builder/handlers_edit.py b/api/core/dify_builder/handlers_edit.py index 67c1a3dfbe9..31ea967cc53 100644 --- a/api/core/dify_builder/handlers_edit.py +++ b/api/core/dify_builder/handlers_edit.py @@ -47,8 +47,10 @@ from core.dify_builder.handlers_fix import ( merge_known_keys, mint_checkpoint, model_config_error_text, + note_repair_error, note_unknown_outcome, perform_revert, + repair_is_repeating, run_finished_without_output, start_schema, testdata_form_fields, @@ -88,6 +90,10 @@ def handle_capability_check(env: Env, turn: Turn, s: Session, fc: DifyBuilderCon text, ok = action_string(turn, "text") if ok and text: fc.goal_text = text + # A new goal is a new repair loop: the breaker's counter must not carry + # over from whatever the previous edit was stuck on. + fc.repair_attempts = 0 + fc.last_repair_error = "" progress = ProgressReporter.for_session( emit=env.emit_progress, @@ -689,6 +695,34 @@ def handle_test_affected_paths(env: Env, turn: Turn, s: Session, fc: DifyBuilder ) progress.activate("edit-diagnose-failure") diagnosis = env.agent.diagnose(run, graph, per_node) + note_repair_error(fc, run) + if repair_is_repeating(fc): + # Same breaker as Build's: the identical engine failure has survived + # MAX_REPEATED_REPAIRS repairs, so another round would aim at the same + # wrong thing. Stop and hand the decision back (no staged repair). + fc.diagnosis = diagnosis + fc.staged_repair = [] + progress.finish() + stuck_items = append_card( + fc, + ErrorCard( + title="Repeated failure", + body=( + f"{diagnosis.root_cause or 'The run failed.'}\n\n" + "The same error survived the last repairs, so I've stopped retrying. " + "Edit the node directly and test again, or revert." + ), + tone="danger", + node_id=diagnosis.culprit_node_id, + ), + ) + return StepResult( + next=PcState.EDIT_AWAIT_REPAIR, + context=fc, + items=stuck_items, + run=run, + run_id_sink=[run.id], + ) progress.activate("edit-prepare-repair") intents, risk = env.agent.propose_repair(diagnosis, graph) fc.diagnosis = diagnosis diff --git a/api/core/dify_builder/handlers_fix.py b/api/core/dify_builder/handlers_fix.py index 0efa47603db..ccd85e70433 100644 --- a/api/core/dify_builder/handlers_fix.py +++ b/api/core/dify_builder/handlers_fix.py @@ -67,6 +67,7 @@ from core.dify_builder.state import PcState logger = logging.getLogger(__name__) __all__ = [ + "MAX_REPEATED_REPAIRS", "MAX_UNKNOWN_OUTCOMES", "NO_OUTPUT_BODY", "NO_OUTPUT_REPLY", @@ -82,6 +83,7 @@ __all__ = [ "dead_end_branch_node_id", "decode_checklist_errors", "emit_canvas", + "failure_signature", "first_failed_node", "fix_registry", "handle_apply", @@ -100,7 +102,9 @@ __all__ = [ "mint_checkpoint", "model_config_error_text", "needs_upload_inputs", + "note_repair_error", "perform_revert", + "repair_is_repeating", "run_finished_without_output", "start_schema", "testdata_form_fields", @@ -216,6 +220,63 @@ def run_finished_without_output(graph: Graph, per_node: list[NodeOutput]) -> boo # ---- helpers --------------------------------------------------------------- +MAX_REPEATED_REPAIRS = 2 + + +def failure_signature(run: Run) -> str: + """A STABLE key for "this is the same failure again", built from the + ENGINE's own output: the first failed node's id plus its error text, or the + launch error when the run threw before any node ran. Whitespace is + collapsed so formatting alone cannot look like a new failure. Returns "" + when the run carries nothing identifying -- an unknown failure must never + compare equal to another unknown one. + + Deliberately NOT ``diagnosis.root_cause``. That is LLM prose, regenerated + on every diagnosis call: in one observed session it was reworded on each of + four turns and switched to Chinese on one of them. Keying on it meant + ``error == fc.last_repair_error`` could never hold, so ``repair_attempts`` + reset forever and ``repair_is_repeating`` was unreachable -- that session + burned 9 failed runs and 8 repair approvals with this guard in place. + """ + for node in run.per_node: + if node.status == "failed": + node_error = " ".join((node.error or "").split()) + if node.node_id or node_error: + return f"{node.node_id}|{node_error}" + launch_error = " ".join((run.error or "").split()) + return f"|{launch_error}" if launch_error else "" + + +def note_repair_error(fc: DifyBuilderContext, run: Run) -> None: + """Record this failure's signature and count CONSECUTIVE repeats of it. + + ``repair_attempts`` is not a global budget: a different failure means the + loop is still making progress, however slowly, so the count restarts. + The same failure again means the repair that just ran did not address the + cause, so the count advances. Call this once per diagnosis, before + ``repair_is_repeating``. + + Takes the ``Run`` rather than a message so the key comes from the engine + (see ``failure_signature``) and not from anything an LLM wrote. + """ + signature = failure_signature(run) + if signature and signature == fc.last_repair_error: + fc.repair_attempts += 1 + else: + fc.repair_attempts = 0 + fc.last_repair_error = signature + + +def repair_is_repeating(fc: DifyBuilderContext) -> bool: + """Has the SAME error now survived ``MAX_REPEATED_REPAIRS`` repairs? + + Reads the counter ``note_repair_error`` maintains; the identical error + that many times over means the repair agent is aiming at something that + isn't the cause, and another round will not find it. + """ + return fc.repair_attempts >= MAX_REPEATED_REPAIRS + + def append_item(fc: DifyBuilderContext, kind: str, payload: dict[str, Any]) -> list[ConversationItem]: """Append a conversation item stamped with the next sequence number. diff --git a/api/tests/unit_tests/core/dify_builder/test_handlers_edit.py b/api/tests/unit_tests/core/dify_builder/test_handlers_edit.py index 84aa4a3b1e2..b797dca88dd 100644 --- a/api/tests/unit_tests/core/dify_builder/test_handlers_edit.py +++ b/api/tests/unit_tests/core/dify_builder/test_handlers_edit.py @@ -1019,3 +1019,44 @@ def test_a_second_consecutive_unknown_outcome_stops_at_the_edit_gate(): assert second.next == PcState.EDIT_AWAIT_REPAIR assert second.context.unknown_outcome_count == 2 assert next(i for i in second.items if i.kind == "error").payload["title"] == "Test outcome unknown" + + +def test_the_same_failing_affected_path_test_trips_the_breaker_on_the_third_repeat(): + """Edit had no repair breaker at all: the same engine failure could be + diagnosed, repaired and re-run indefinitely. It now shares Build's.""" + from core.dify_builder.handlers_edit import handle_test_affected_paths + from core.dify_builder.handlers_fix import MAX_REPEATED_REPAIRS + from core.dify_builder.models import TestInput + + dify = FakeEditDifyPort() + dify.verify_pass = False # the engine fails the same way every time + env, repo = _new_env(dify=dify, agent=StubAgent()) + s = _session(entry_mode=EntryMode.EDIT, current_state=PcState.EDIT_TEST_AFFECTED_PATHS) + repo.save_test_input(TestInput(id="ti-1", session_id=s.id, source="mock", inputs={})) + fc = DifyBuilderContext(test_input_ref="ti-1") + + snapshots = [] # the handler mutates the SAME context object; snapshot per call + result = None + for _ in range(MAX_REPEATED_REPAIRS + 1): + result = handle_test_affected_paths(env, Turn(actor=_actor()), s, fc) + fc = result.context + snapshots.append((fc.repair_attempts, len(fc.staged_repair))) + + assert snapshots == [(0, 1), (1, 1), (2, 0)] + assert result.next == PcState.EDIT_AWAIT_REPAIR + assert next(i for i in result.items if i.kind == "error").payload["title"] == "Repeated failure" + + +def test_sending_a_new_edit_goal_resets_the_breaker(): + from core.dify_builder.handlers_edit import handle_capability_check + + env, repo = _new_env() + s = _seed_edit_session(repo, PcState.EDIT_CAPABILITY_CHECK, repair_attempts=3, last_repair_error="llm|boom") + turn = Turn( + action=Action(kind="send_edit_goal", payload={"text": "Make it faster"}, base_version=1), actor=_actor() + ) + + res = handle_capability_check(env, turn, *repo.get_session(s.id)) + + assert res.context.repair_attempts == 0 + assert res.context.last_repair_error == ""