mirror of
https://github.com/MervinPraison/PraisonAI.git
synced 2026-11-07 04:35:58 -10:00
fix: make completion promise the only autonomy success signal (#6064)
* fix: make completion promise the only autonomy success signal (fixes #6027) When a completion_promise is configured, the tool_completion, keyword, direct-stage and no_tool_calls heuristics no longer end the run with success=True in run_autonomous/run_autonomous_async. Promise-less runs are unchanged. The async tool path now increments _autonomy_turn_tool_count for sync/async parity, and `praisonai loop` sets mode="iterative" so --completion-promise/--max-iterations take effect without --level full_auto. Co-Authored-By: Claude Opus 5.5 <[email protected]> * fix: tell the model the completion promise and only count executed async tools Addresses review on #6064: SDK autonomy loops now append the expected <promise> tag to the prompt (unless already present in the prompt or instructions), and the async per-turn tool count is incremented only after the tool runs so BEFORE_TOOL-blocked calls cannot trigger tool_completion. Co-Authored-By: Claude Opus 5.5 <[email protected]> --------- Co-authored-by: MervinPraison <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]>
This commit is contained in:
6 files changed
+186
-13
No files matched your search
@@ -4770,6 +4770,11 @@ Summary:"""
|
||||
effective_promise = completion_promise
|
||||
if effective_promise is None:
|
||||
effective_promise = self.autonomy_config.get("completion_promise")
|
||||
# The promise is the only success signal, so tell the model about it
|
||||
# unless the caller already did (e.g. the `loop` CLI instructions).
|
||||
_promise_tag = f"<promise>{effective_promise}</promise>"
|
||||
if effective_promise and _promise_tag not in prompt and _promise_tag not in (self.instructions or ""):
|
||||
prompt = f"{prompt}\n\nWhen the task is fully complete, include {_promise_tag} in your response."
|
||||
|
||||
# Get clear_context from config if not explicitly set
|
||||
effective_clear_context = clear_context
|
||||
@@ -5012,7 +5017,7 @@ Summary:"""
|
||||
# the task naturally (model stopped calling tools = done).
|
||||
# This is the same signal the CLI/wrapper path trusts.
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
if self._autonomy_turn_tool_count > 0 and len(response_str) > 100:
|
||||
if not effective_promise and self._autonomy_turn_tool_count > 0 and len(response_str) > 100:
|
||||
# P3/G2: Emit completion callback
|
||||
execute_sync_callback('autonomy_complete',
|
||||
completion_reason="tool_completion",
|
||||
@@ -5049,7 +5054,8 @@ Summary:"""
|
||||
if obs is not None:
|
||||
obs.emit(_EvtType.STAGE_ESCALATE, {"from": prev_stage, "to": stage})
|
||||
|
||||
# Check for completion promise FIRST (structured signal)
|
||||
# Completion promise: when configured, it is the only success signal
|
||||
# (heuristic exits above/below are skipped in promise mode)
|
||||
if effective_promise:
|
||||
promise_tag = f"<promise>{effective_promise}</promise>"
|
||||
if promise_tag in response_str:
|
||||
@@ -5071,7 +5077,7 @@ Summary:"""
|
||||
)
|
||||
|
||||
# Check for keyword-based completion signals (word-boundary, G-COMPLETION-1 fix)
|
||||
if self._is_completion_signal(response_str):
|
||||
if not effective_promise and self._is_completion_signal(response_str):
|
||||
# P3/G2: Emit completion callback
|
||||
execute_sync_callback('autonomy_complete',
|
||||
completion_reason="goal",
|
||||
@@ -5090,7 +5096,7 @@ Summary:"""
|
||||
)
|
||||
|
||||
# For DIRECT stage, complete after first response
|
||||
if stage == "direct":
|
||||
if not effective_promise and stage == "direct":
|
||||
# P3/G2: Emit completion callback
|
||||
execute_sync_callback('autonomy_complete',
|
||||
completion_reason="goal",
|
||||
@@ -5118,7 +5124,7 @@ Summary:"""
|
||||
# Suppressed when doom recovery is active — once a doom loop
|
||||
# is detected, no_tool_calls (a success exit) should not fire;
|
||||
# the doom loop system manages termination instead.
|
||||
if (self._autonomy_turn_tool_count == 0 and iterations > 1
|
||||
if (not effective_promise and self._autonomy_turn_tool_count == 0 and iterations > 1
|
||||
and not self._doom_recovery_active):
|
||||
self._consecutive_no_tool_turns += 1
|
||||
if self._consecutive_no_tool_turns >= 2:
|
||||
@@ -5257,6 +5263,11 @@ Summary:"""
|
||||
effective_promise = completion_promise
|
||||
if effective_promise is None:
|
||||
effective_promise = self.autonomy_config.get("completion_promise")
|
||||
# The promise is the only success signal, so tell the model about it
|
||||
# unless the caller already did (e.g. the `loop` CLI instructions).
|
||||
_promise_tag = f"<promise>{effective_promise}</promise>"
|
||||
if effective_promise and _promise_tag not in prompt and _promise_tag not in (self.instructions or ""):
|
||||
prompt = f"{prompt}\n\nWhen the task is fully complete, include {_promise_tag} in your response."
|
||||
|
||||
# Get clear_context from config if not explicitly set
|
||||
effective_clear_context = clear_context
|
||||
@@ -5479,7 +5490,7 @@ Summary:"""
|
||||
# produced a substantive response, the inner loop completed
|
||||
# the task naturally (model stopped calling tools = done).
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
if self._autonomy_turn_tool_count > 0 and len(response_str) > 100:
|
||||
if not effective_promise and self._autonomy_turn_tool_count > 0 and len(response_str) > 100:
|
||||
return AutonomyResult(
|
||||
success=True,
|
||||
output=response_str,
|
||||
@@ -5502,7 +5513,8 @@ Summary:"""
|
||||
if obs is not None:
|
||||
obs.emit(_EvtType.STAGE_ESCALATE, {"from": stage_order[idx], "to": stage})
|
||||
|
||||
# Check for completion promise FIRST (structured signal)
|
||||
# Completion promise: when configured, it is the only success signal
|
||||
# (heuristic exits above/below are skipped in promise mode)
|
||||
if effective_promise:
|
||||
promise_tag = f"<promise>{effective_promise}</promise>"
|
||||
if promise_tag in response_str:
|
||||
@@ -5518,7 +5530,7 @@ Summary:"""
|
||||
)
|
||||
|
||||
# Check for keyword-based completion signals (word-boundary, G-COMPLETION-1 fix)
|
||||
if self._is_completion_signal(response_str):
|
||||
if not effective_promise and self._is_completion_signal(response_str):
|
||||
return AutonomyResult(
|
||||
success=True,
|
||||
output=response_str,
|
||||
@@ -5531,7 +5543,7 @@ Summary:"""
|
||||
)
|
||||
|
||||
# For DIRECT stage, complete after first response
|
||||
if stage == "direct":
|
||||
if not effective_promise and stage == "direct":
|
||||
return AutonomyResult(
|
||||
success=True,
|
||||
output=response_str,
|
||||
@@ -5552,7 +5564,7 @@ Summary:"""
|
||||
# Suppressed when doom recovery is active — once a doom loop
|
||||
# is detected, no_tool_calls (a success exit) should not fire;
|
||||
# the doom loop system manages termination instead.
|
||||
if (self._autonomy_turn_tool_count == 0 and iterations > 1
|
||||
if (not effective_promise and self._autonomy_turn_tool_count == 0 and iterations > 1
|
||||
and not self._doom_recovery_active):
|
||||
self._consecutive_no_tool_turns += 1
|
||||
if self._consecutive_no_tool_turns >= 2:
|
||||
|
||||
@@ -1427,6 +1427,10 @@ Write the complete compiled report:"""
|
||||
_loop_detection.record_tool_outcome(
|
||||
_ld_history, function_name, arguments, _result, _ld_config
|
||||
)
|
||||
# Per-turn tool count for autonomy completion heuristics (sync
|
||||
# parity with tool_execution.py). Counted only once the tool has
|
||||
# actually run, so a BEFORE_TOOL-blocked call cannot count.
|
||||
self._autonomy_turn_tool_count = getattr(self, '_autonomy_turn_tool_count', 0) + 1
|
||||
return _result
|
||||
|
||||
# Enforce BEFORE_TOOL/AFTER_TOOL security hooks for every async caller,
|
||||
|
||||
@@ -0,0 +1,130 @@
|
||||
"""A configured completion promise must be the only success signal (#6027)."""
|
||||
|
||||
import asyncio
|
||||
|
||||
import pytest
|
||||
|
||||
from praisonaiagents import Agent
|
||||
from praisonaiagents.model_harness import ScriptedModel
|
||||
|
||||
NOT_DONE = (
|
||||
"I listed the files and started on the parser, but work remains: the tokenizer "
|
||||
"is still failing two tests and the CLI flag is not wired yet. Continuing next turn."
|
||||
)
|
||||
DONE_REPLY = "Fixed the tokenizer and wired the flag. <promise>DONE</promise>"
|
||||
PROMPT = "Refactor auth module" # stage=autonomous
|
||||
DIRECT_PROMPT = "Implement feature X" # stage=direct
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _offline(monkeypatch, tmp_path):
|
||||
for key in ("OPENAI_BASE_URL", "OPENAI_API_BASE", "ANTHROPIC_BASE_URL"):
|
||||
monkeypatch.setenv(key, "http://127.0.0.1:9")
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
|
||||
|
||||
def list_files(path: str) -> str:
|
||||
"""List files in a directory."""
|
||||
return "parser.py tokenizer.py cli.py"
|
||||
|
||||
|
||||
def _script(follow_ups=1):
|
||||
return (
|
||||
[ScriptedModel.tool_call("list_files", {"path": "."})]
|
||||
+ [NOT_DONE] * follow_ups
|
||||
+ [DONE_REPLY] * 3
|
||||
)
|
||||
|
||||
|
||||
def _make(model, **autonomy):
|
||||
return Agent(
|
||||
name="worker", instructions="Do the task.", llm=model, tools=[list_files],
|
||||
output="silent", autonomy={"level": "full_auto", **autonomy},
|
||||
)
|
||||
|
||||
|
||||
def _assert_promise(result):
|
||||
assert result.success is True
|
||||
assert result.completion_reason == "promise"
|
||||
assert "<promise>DONE</promise>" in result.output
|
||||
assert result.iterations >= 2
|
||||
|
||||
|
||||
def test_sync_tool_completion_does_not_preempt_promise():
|
||||
r = _make(ScriptedModel(_script())).run_autonomous(
|
||||
PROMPT, completion_promise="DONE", max_iterations=5)
|
||||
_assert_promise(r)
|
||||
|
||||
|
||||
def test_async_waits_for_promise():
|
||||
r = asyncio.run(_make(ScriptedModel(_script(2))).run_autonomous_async(
|
||||
PROMPT, completion_promise="DONE", max_iterations=5))
|
||||
_assert_promise(r)
|
||||
|
||||
|
||||
def test_sync_direct_stage_waits_for_promise():
|
||||
r = _make(ScriptedModel([NOT_DONE, DONE_REPLY])).run_autonomous(
|
||||
DIRECT_PROMPT, completion_promise="DONE", max_iterations=5)
|
||||
_assert_promise(r)
|
||||
|
||||
|
||||
def test_async_direct_stage_waits_for_promise():
|
||||
r = asyncio.run(_make(ScriptedModel([NOT_DONE, DONE_REPLY])).run_autonomous_async(
|
||||
DIRECT_PROMPT, completion_promise="DONE", max_iterations=5))
|
||||
_assert_promise(r)
|
||||
|
||||
|
||||
def test_promise_never_emitted_ends_unsuccessfully():
|
||||
r = _make(ScriptedModel([NOT_DONE] * 5)).run_autonomous(
|
||||
PROMPT, completion_promise="DONE", max_iterations=3)
|
||||
assert r.success is False
|
||||
assert r.completion_reason == "max_iterations"
|
||||
|
||||
|
||||
def test_iterative_mode_start_honours_promise():
|
||||
agent = Agent(
|
||||
name="loop_agent", instructions="Complete the task.", llm=ScriptedModel(_script()),
|
||||
tools=[list_files], output="silent",
|
||||
autonomy={"mode": "iterative", "max_iterations": 10,
|
||||
"completion_promise": "DONE", "observe": False},
|
||||
)
|
||||
_assert_promise(agent.start(PROMPT))
|
||||
|
||||
|
||||
def test_without_promise_tool_completion_unchanged():
|
||||
r = _make(ScriptedModel(_script())).run_autonomous(PROMPT, max_iterations=5)
|
||||
assert r.success is True
|
||||
assert r.completion_reason == "tool_completion"
|
||||
assert r.iterations == 1
|
||||
|
||||
|
||||
def test_async_tool_path_counts_tools():
|
||||
r = asyncio.run(_make(ScriptedModel(_script(2))).run_autonomous_async(
|
||||
PROMPT, max_iterations=5))
|
||||
assert r.success is True
|
||||
assert r.completion_reason == "tool_completion"
|
||||
assert r.iterations == 1
|
||||
|
||||
|
||||
def test_promise_tag_is_sent_to_model():
|
||||
model = ScriptedModel([DONE_REPLY])
|
||||
_make(model).run_autonomous(PROMPT, completion_promise="DONE", max_iterations=2)
|
||||
assert "<promise>DONE</promise>" in model.requests[0].last_user_message
|
||||
|
||||
|
||||
def test_async_blocked_tool_does_not_count_as_tool_completion():
|
||||
from praisonaiagents.hooks import HookEvent, HookRegistry, HookResult
|
||||
|
||||
registry = HookRegistry()
|
||||
|
||||
@registry.on(HookEvent.BEFORE_TOOL)
|
||||
def deny(data):
|
||||
return HookResult.deny("blocked")
|
||||
|
||||
agent = Agent(
|
||||
name="worker", instructions="Do the task.", llm=ScriptedModel(_script(2)),
|
||||
tools=[list_files], output="silent", hooks=registry,
|
||||
autonomy={"level": "full_auto"},
|
||||
)
|
||||
r = asyncio.run(agent.run_autonomous_async(PROMPT, max_iterations=3))
|
||||
assert r.completion_reason != "tool_completion"
|
||||
@@ -154,9 +154,14 @@ class TestPromptReinjection:
|
||||
completion_promise="DONE"
|
||||
)
|
||||
|
||||
# All prompts should be the original, not "Continue with the task"
|
||||
# All prompts should be the original (plus the promise instruction),
|
||||
# not "Continue with the task"
|
||||
expected = (
|
||||
"Build a REST API\n\nWhen the task is fully complete, include "
|
||||
"<promise>DONE</promise> in your response."
|
||||
)
|
||||
for prompt in prompts_received:
|
||||
assert prompt == "Build a REST API", f"Expected original prompt, got: {prompt}"
|
||||
assert prompt == expected, f"Expected original prompt, got: {prompt}"
|
||||
|
||||
def test_prompt_not_continue_with_task(self):
|
||||
"""Prompt should NOT be 'Continue with the task' after first iteration."""
|
||||
|
||||
@@ -88,8 +88,11 @@ def _run_loop(
|
||||
typer.echo("Error: praisonaiagents not installed. Run: pip install praisonaiagents", err=True)
|
||||
raise typer.Exit(1)
|
||||
|
||||
# Build autonomy config
|
||||
# Build autonomy config. `loop` always iterates: without an explicit mode
|
||||
# the default level (suggest) maps to single-call "caller" mode, which
|
||||
# would ignore --completion-promise and --max-iterations.
|
||||
autonomy_config = {
|
||||
"mode": "iterative",
|
||||
"max_iterations": max_iterations,
|
||||
"completion_promise": completion_promise,
|
||||
"clear_context": clear_context,
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
"""`praisonai loop` must run the iterative autonomy loop by default (#6027)."""
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from praisonai_code.cli.commands.loop import _run_loop
|
||||
|
||||
|
||||
def test_loop_builds_iterative_autonomy_config():
|
||||
result = MagicMock(success=True, completion_reason="promise", iterations=2,
|
||||
duration_seconds=0.1, started_at=None)
|
||||
with patch("praisonaiagents.Agent") as agent_cls:
|
||||
agent_cls.return_value.start.return_value = result
|
||||
_run_loop("Refactor auth module", max_iterations=10, completion_promise="DONE",
|
||||
clear_context=False, timeout=None, model=None, verbose=False)
|
||||
|
||||
autonomy = agent_cls.call_args.kwargs["autonomy"]
|
||||
assert autonomy["mode"] == "iterative"
|
||||
assert autonomy["completion_promise"] == "DONE"
|
||||
assert autonomy["max_iterations"] == 10
|
||||
Reference in new issue
Block a user