mirror of
https://github.com/langchain-ai/deepagents.git
synced 2026-07-21 17:25:26 -04:00
fix(code): avoid duplicate "criteria ready" message on /goal revisions (#4559)
Fixed `/goal` showing a duplicate "Proposed acceptance criteria are ready." message when revising proposed criteria. --- When using `/goal` and going through revision cycles of the proposed criteria, `_propose_goal_rubric` re-announced "Proposed acceptance criteria are ready." on every regeneration, so the transcript accumulated a duplicate message each cycle. This suppresses the success message when regenerating from feedback (the review widget is remounted with the updated criteria anyway) while still surfacing any unsaved-state warning. Made by [Open SWE](https://openswe.vercel.app/agents/2b61a2fb-628a-5e98-6e33-5cc275d5d42a) --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
@@ -8368,15 +8368,22 @@ class DeepAgentsApp(App):
|
||||
return False
|
||||
return True
|
||||
|
||||
async def _mount_goal_rubric_result(self, message: str, *, persisted: bool) -> None:
|
||||
async def _mount_goal_rubric_result(
|
||||
self, message: str, *, persisted: bool, suppress_success: bool = False
|
||||
) -> None:
|
||||
"""Mount a goal/rubric command result, flagging unsaved state.
|
||||
|
||||
Args:
|
||||
message: Success text describing the applied change.
|
||||
persisted: Whether `_persist_goal_rubric_state` confirmed the write.
|
||||
suppress_success: When `True`, skip the success message on a
|
||||
persisted write but still surface the unsaved-state warning.
|
||||
Used by revision cycles that would otherwise duplicate a
|
||||
message already shown for the first proposal.
|
||||
"""
|
||||
if persisted:
|
||||
await self._mount_message(AppMessage(message))
|
||||
if not suppress_success:
|
||||
await self._mount_message(AppMessage(message))
|
||||
return
|
||||
await self._mount_message(
|
||||
ErrorMessage(
|
||||
@@ -8943,9 +8950,14 @@ class DeepAgentsApp(App):
|
||||
self._pending_goal_objective = objective
|
||||
self._pending_goal_rubric = rubric
|
||||
persisted = await self._persist_goal_rubric_state()
|
||||
# On a revision the remounted review widget already shows the updated
|
||||
# criteria, so re-announcing readiness is redundant and would duplicate
|
||||
# the message from the first proposal. The unsaved-state warning still
|
||||
# surfaces regardless (see `_mount_goal_rubric_result`).
|
||||
await self._mount_goal_rubric_result(
|
||||
"Proposed acceptance criteria are ready.",
|
||||
persisted=persisted,
|
||||
suppress_success=feedback is not None,
|
||||
)
|
||||
await self._start_pending_goal_rubric_review()
|
||||
|
||||
|
||||
@@ -5772,6 +5772,72 @@ class TestGoalCommand:
|
||||
assert app._pending_goal_objective == "add refresh tokens"
|
||||
assert app._pending_goal_rubric == "- model draft"
|
||||
|
||||
async def test_goal_revision_does_not_duplicate_ready_message(self) -> None:
|
||||
"""Revision cycles must not re-announce readiness (see duplicate bug)."""
|
||||
app = DeepAgentsApp(agent=MagicMock())
|
||||
async with app.run_test() as pilot:
|
||||
await pilot.pause()
|
||||
# Stub the review continuation so no goal-review task is scheduled;
|
||||
# the dedup under test runs before it in `_propose_goal_rubric`.
|
||||
with (
|
||||
patch.object(app, "_generate_goal_rubric", return_value="- tests pass"),
|
||||
patch.object(
|
||||
app, "_start_pending_goal_rubric_review", new_callable=AsyncMock
|
||||
),
|
||||
patch.object(app, "_set_spinner", new_callable=AsyncMock),
|
||||
):
|
||||
await app._propose_goal_rubric("add refresh tokens")
|
||||
await pilot.pause()
|
||||
await app._propose_goal_rubric(
|
||||
"add refresh tokens",
|
||||
feedback="be thorough in research",
|
||||
previous_criteria="- tests pass",
|
||||
)
|
||||
await pilot.pause()
|
||||
|
||||
ready_messages = [
|
||||
w
|
||||
for w in app.query(AppMessage)
|
||||
if str(w._content) == "Proposed acceptance criteria are ready."
|
||||
]
|
||||
assert len(ready_messages) == 1
|
||||
|
||||
async def test_goal_revision_warns_when_persist_fails_despite_suppression(
|
||||
self,
|
||||
) -> None:
|
||||
"""A revision that cannot be saved must still surface the unsaved warning."""
|
||||
app = DeepAgentsApp(agent=MagicMock())
|
||||
async with app.run_test() as pilot:
|
||||
await pilot.pause()
|
||||
app._agent = SimpleNamespace(
|
||||
aupdate_state=AsyncMock(side_effect=RuntimeError("down"))
|
||||
)
|
||||
app._lc_thread_id = "thread-1"
|
||||
with (
|
||||
patch.object(app, "_generate_goal_rubric", return_value="- tests pass"),
|
||||
patch.object(
|
||||
app, "_start_pending_goal_rubric_review", new_callable=AsyncMock
|
||||
),
|
||||
patch.object(app, "_set_spinner", new_callable=AsyncMock),
|
||||
):
|
||||
await app._propose_goal_rubric(
|
||||
"add refresh tokens",
|
||||
feedback="be thorough in research",
|
||||
previous_criteria="- old criteria",
|
||||
)
|
||||
await pilot.pause()
|
||||
|
||||
# Success text is suppressed on the revision, but the failed write
|
||||
# must still warn the user that the change is unsaved.
|
||||
assert not any(
|
||||
str(w._content) == "Proposed acceptance criteria are ready."
|
||||
for w in app.query(AppMessage)
|
||||
)
|
||||
assert any(
|
||||
"could not be saved to the thread" in str(w._content)
|
||||
for w in app.query(ErrorMessage)
|
||||
)
|
||||
|
||||
async def test_goal_review_regeneration_failure_remounts_pending_review(
|
||||
self,
|
||||
) -> None:
|
||||
@@ -5871,7 +5937,13 @@ class TestGoalCommand:
|
||||
thread_id="thread-1",
|
||||
preloaded_payload=payload,
|
||||
)
|
||||
await pilot.pause()
|
||||
# The review task is scheduled via `call_after_refresh`, which
|
||||
# can take more than one refresh to fire; wait for it instead of
|
||||
# racing a single pause (previously flaky under event-loop load).
|
||||
for _ in range(50):
|
||||
await pilot.pause()
|
||||
if app._goal_review_task is not None:
|
||||
break
|
||||
|
||||
menu = app.query_one(GoalReviewMenu)
|
||||
assert app._pending_goal_review_widget is menu
|
||||
|
||||
Reference in New Issue
Block a user