mirror of
https://github.com/langgenius/dify.git
synced 2026-10-04 13:47:55 +08:00
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) <noreply@anthropic.com>
This commit is contained in:
1 parent
ccb217d72f
commit
b9ee13f0a9
4 files changed
+146
-55
No files matched your search
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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 == ""
|
||||
Reference in new issue
Block a user