-
Notifications
You must be signed in to change notification settings - Fork 2.9k
fix(agent): restore ReAct multi-turn user history (#3171) #3198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
mumubuku
wants to merge
8
commits into
eosphoros-ai:main
Choose a base branch
from
mumubuku:fix/react-agent-multi-turn-memory
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
ab4bf06
fix(agent): pass prior ReAct user turns into follow-up prompts (#3171)
mumubuku 875a9f2
fix(agent): fold prior ReAct turns into the current Observation (#3171)
mumubuku 590ec4f
fix(agent): skip empty ReAct final_content instead of dumping tool JSON
mumubuku fe20ff3
revert: leave ConversableAgent historical_dialogues behavior unchanged
mumubuku 86e8c39
docs: add multi-chart tab Run screenshots for PR 3199
mumubuku 097f087
docs: add PR 3198 verification screenshots
mumubuku af2583a
docs: drop verification screenshots from tree after attaching to PR
mumubuku d52112b
docs: add live ReAct multi-turn UI proof for #3171
mumubuku File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
136 changes: 136 additions & 0 deletions
136
packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/react_history.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| """Build prior-turn ReAct history for a follow-up request. | ||
|
|
||
| Each HTTP call to ``chat_react_agent`` constructs a new ``ReActAgent`` whose | ||
| short-term memory starts empty. Tool-step fragments recovered from GPTs | ||
| message storage (if any) are also truncated by a 5-slot buffer, so the | ||
| previous *user question* and *final answer* are usually missing from the | ||
| LLM prompt. | ||
|
|
||
| ReAct's thinking step treats the last Human message as the task | ||
| (``Observation: ...``). Extra Human/AI turns can be ignored or even | ||
| re-executed as a new task. Completed turns are therefore folded into that | ||
| single current question, with only ``final_content`` (not prior SQL traces). | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json | ||
| import logging | ||
| from typing import Any, Iterable, List, Optional, Sequence, Tuple | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| DEFAULT_MAX_TURNS = 8 | ||
| DEFAULT_MAX_ANSWER_CHARS = 4000 | ||
|
|
||
| _HISTORY_HINT = """ | ||
| ## Previous conversation | ||
| When the user message contains "Prior turns in this conversation", those | ||
| turns already happened in this same chat. Use them to resolve references | ||
| such as "上面", "刚才", "the order above", or "that product". Answer the | ||
| ## Current question only; do not redo a prior turn unless the user asks. | ||
| """ | ||
|
|
||
|
|
||
| def extract_react_final_content( | ||
| raw: str, max_chars: int = DEFAULT_MAX_ANSWER_CHARS | ||
| ) -> str: | ||
| """Prefer ``final_content`` from a persisted react-agent view payload.""" | ||
| text = (raw or "").strip() | ||
| if text.startswith("{") and "final_content" in text: | ||
| try: | ||
| payload = json.loads(text) | ||
| if isinstance(payload, dict) and "final_content" in payload: | ||
| final_content = payload["final_content"] | ||
| text = str(final_content).strip() if final_content is not None else "" | ||
| except (TypeError, ValueError, json.JSONDecodeError): | ||
| logger.debug("Failed to parse react-agent view payload as JSON") | ||
| if max_chars > 0 and len(text) > max_chars: | ||
| if max_chars <= 3: | ||
| text = text[:max_chars] | ||
| else: | ||
| text = text[: max_chars - 3] + "..." | ||
| return text | ||
|
|
||
|
|
||
| def _message_role(msg: Any) -> str: | ||
| role = getattr(msg, "type", None) or getattr(msg, "role", None) or "" | ||
| return str(role).lower() | ||
|
|
||
|
|
||
| def _message_text(msg: Any) -> str: | ||
| last_text = getattr(msg, "last_text", None) | ||
| if isinstance(last_text, str): | ||
| return last_text.strip() | ||
| content = getattr(msg, "content", "") | ||
| if isinstance(content, str): | ||
| return content.strip() | ||
| return str(content or "").strip() | ||
|
|
||
|
|
||
| def completed_turn_pairs( | ||
| messages: Optional[Iterable[Any]], | ||
| current_user_input: str, | ||
| max_turns: int = DEFAULT_MAX_TURNS, | ||
| max_answer_chars: int = DEFAULT_MAX_ANSWER_CHARS, | ||
| ) -> List[Tuple[str, str]]: | ||
| """Return ``(question, final_answer)`` for completed prior turns.""" | ||
| current = (current_user_input or "").strip() | ||
| pairs: List[Tuple[str, str]] = [] | ||
| pending_q: Optional[str] = None | ||
| for msg in messages or []: | ||
| role = _message_role(msg) | ||
| text = _message_text(msg) | ||
| if not text: | ||
| continue | ||
| if role == "human": | ||
| pending_q = text | ||
| continue | ||
| if role in ("view", "ai") and pending_q: | ||
| answer = extract_react_final_content(text, max_answer_chars) | ||
| if pending_q and answer: | ||
| pairs.append((pending_q, answer)) | ||
| pending_q = None | ||
| # The current round's user message is appended before the agent runs. | ||
| if pending_q is not None and pending_q == current: | ||
| pending_q = None | ||
| if max_turns > 0: | ||
| pairs = pairs[-max_turns:] | ||
| return pairs | ||
|
|
||
|
|
||
| def format_react_followup_question( | ||
| current_user_input: str, | ||
| messages: Optional[Sequence[Any]] = None, | ||
| max_turns: int = DEFAULT_MAX_TURNS, | ||
| max_answer_chars: int = DEFAULT_MAX_ANSWER_CHARS, | ||
| ) -> str: | ||
| """Fold prior Q/A into the current user text (the ReAct Observation). | ||
|
|
||
| First-turn requests with no completed history are returned unchanged so | ||
| stored ``user_input`` and the model question stay aligned. | ||
| """ | ||
| current = current_user_input or "" | ||
| pairs = completed_turn_pairs( | ||
| messages, | ||
| current, | ||
| max_turns=max_turns, | ||
| max_answer_chars=max_answer_chars, | ||
| ) | ||
| if not pairs: | ||
| return current | ||
| lines = ["## Prior turns in this conversation"] | ||
| for idx, (question, answer) in enumerate(pairs, start=1): | ||
| lines.append(f"Turn {idx} user: {question}") | ||
| lines.append(f"Turn {idx} assistant: {answer}") | ||
| lines.append("") | ||
| lines.append("## Current question") | ||
| lines.append(current) | ||
| return "\n".join(lines) | ||
|
|
||
|
|
||
| def history_prompt_hint(has_prior_turns: bool) -> str: | ||
| """Extra system-prompt paragraph when prior turns are present.""" | ||
| if not has_prior_turns: | ||
| return "" | ||
| return _HISTORY_HINT |
Empty file.
131 changes: 131 additions & 0 deletions
131
packages/dbgpt-app/src/dbgpt_app/openapi/api_v1/tests/test_react_history.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| """Tests for ReAct multi-turn conversation history (#3171).""" | ||
|
|
||
| import json | ||
|
|
||
| from dbgpt.core.interface.message import AIMessage, HumanMessage, ViewMessage | ||
| from dbgpt_app.openapi.api_v1.react_history import ( | ||
| completed_turn_pairs, | ||
| extract_react_final_content, | ||
| format_react_followup_question, | ||
| history_prompt_hint, | ||
| ) | ||
|
|
||
| # Issue #3171 reproduction: turn 2 refers to "上面" the max-GMV consumed order. | ||
| TURN1_Q = "查询26年单次已经消费gmv最大的订单是哪个?" | ||
| TURN1_A = "order_id=3, sku_id=100, gmv=20(consume_status=1)" | ||
| TURN2_Q = "统计和上面单次消费gmv最大订单所属商品的总计gmv有多少?" | ||
|
|
||
|
|
||
| def _turn1_view_payload(final_content: str = TURN1_A) -> str: | ||
| return json.dumps( | ||
| { | ||
| "version": 1, | ||
| "type": "react-agent", | ||
| "final_content": final_content, | ||
| "steps": [ | ||
| {"action": "sql_query", "action_input": '{"sql": "SELECT ..."}'}, | ||
| ], | ||
| }, | ||
| ensure_ascii=False, | ||
| ) | ||
|
|
||
|
|
||
| def _issue_3171_messages(): | ||
| return [ | ||
| HumanMessage(content=TURN1_Q), | ||
| ViewMessage(content=_turn1_view_payload()), | ||
| HumanMessage(content=TURN2_Q), | ||
| ] | ||
|
|
||
|
|
||
| def test_extract_prefers_final_content_and_drops_tool_steps(): | ||
| payload = _turn1_view_payload("order_id=3, sku_id=100") | ||
| assert extract_react_final_content(payload) == "order_id=3, sku_id=100" | ||
| assert "sql_query" not in extract_react_final_content(payload) | ||
|
|
||
|
|
||
| def test_extract_keeps_plain_text_and_truncates(): | ||
| assert extract_react_final_content("hello") == "hello" | ||
| assert extract_react_final_content("abcdef", max_chars=5) == "ab..." | ||
| assert len(extract_react_final_content("abcdef", max_chars=1)) <= 1 | ||
| assert len(extract_react_final_content("abcdef", max_chars=2)) <= 2 | ||
| assert extract_react_final_content("abcdef", max_chars=1) == "a" | ||
| assert extract_react_final_content("abcdef", max_chars=2) == "ab" | ||
|
|
||
|
|
||
| def test_empty_final_content_does_not_inject_tool_steps(): | ||
| payload = json.dumps( | ||
| { | ||
| "version": 1, | ||
| "type": "react-agent", | ||
| "final_content": "", | ||
| "steps": [{"action": "sql_query", "action_input": '{"sql": "SELECT 1"}'}], | ||
| }, | ||
| ensure_ascii=False, | ||
| ) | ||
| assert extract_react_final_content(payload) == "" | ||
| messages = [ | ||
| HumanMessage(content="q1"), | ||
| ViewMessage(content=payload), | ||
| HumanMessage(content="q2"), | ||
| ] | ||
| assert format_react_followup_question("q2", messages) == "q2" | ||
| assert completed_turn_pairs(messages, "q2") == [] | ||
|
|
||
|
|
||
| def test_issue_3171_followup_observation_contains_sku_and_current_question(): | ||
| """What the ReAct LLM actually attends to is the last Human Observation.""" | ||
| followup = format_react_followup_question(TURN2_Q, _issue_3171_messages()) | ||
| observation = f"Observation: {followup}" | ||
|
|
||
| assert "sku_id=100" in observation | ||
| assert "order_id=3" in observation | ||
| assert TURN1_Q in observation | ||
| assert TURN2_Q in observation | ||
| assert observation.strip().endswith(TURN2_Q) | ||
| # Tool traces from turn 1 must not flood the follow-up. | ||
| assert "sql_query" not in observation | ||
| # First-turn current question is labeled so the agent does not re-solve it. | ||
| assert "## Current question" in observation | ||
| assert history_prompt_hint(True) | ||
| assert history_prompt_hint(False) == "" | ||
|
|
||
|
|
||
| def test_first_turn_is_unchanged(): | ||
| assert format_react_followup_question("hello", [HumanMessage(content="hello")]) == ( | ||
| "hello" | ||
| ) | ||
|
|
||
|
|
||
| def test_storage_conversation_matches_react_stream_order(): | ||
| """Mirror _react_agent_stream: persist turn 1, then append turn 2 user input.""" | ||
| from dbgpt.core.interface.message import StorageConversation | ||
|
|
||
| conv = StorageConversation(conv_uid="issue-3171", chat_mode="chat_react_agent") | ||
| conv.start_new_round() | ||
| conv.add_user_message(TURN1_Q) | ||
| conv.add_view_message(_turn1_view_payload()) | ||
| conv.end_current_round() | ||
| conv.start_new_round() | ||
| conv.add_user_message(TURN2_Q) | ||
|
|
||
| followup = format_react_followup_question(TURN2_Q, conv.messages) | ||
| assert followup != TURN2_Q | ||
| assert "sku_id=100" in followup | ||
| assert followup.endswith(TURN2_Q) | ||
|
|
||
|
|
||
| def test_completed_pairs_keep_last_n_turns_and_accept_ai_role(): | ||
| messages = [ | ||
| HumanMessage(content="q1"), | ||
| AIMessage(content="a1"), | ||
| HumanMessage(content="q2"), | ||
| ViewMessage(content="a2"), | ||
| HumanMessage(content="q3"), | ||
| ViewMessage(content="a3"), | ||
| HumanMessage(content="now"), | ||
| ] | ||
| assert completed_turn_pairs(messages, "now", max_turns=2) == [ | ||
| ("q2", "a2"), | ||
| ("q3", "a3"), | ||
| ] | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover short truncation limits.
extract_react_final_content("abcdef", max_chars=1)currently returns"abcd...". This exceedsmax_charsbecause the implementation uses a negative slice index. The current test only coversmax_chars=5, so it does not detect this boundary failure. Add regression cases formax_chars=1andmax_chars=2, then correct the truncation implementation.Proposed regression coverage
def test_extract_keeps_plain_text_and_truncates(): assert extract_react_final_content("hello") == "hello" assert extract_react_final_content("abcdef", max_chars=5) == "ab..." + assert len(extract_react_final_content("abcdef", max_chars=1)) <= 1 + assert len(extract_react_final_content("abcdef", max_chars=2)) <= 2As per path instructions, cover relevant boundary behavior.
Source: Path instructions