mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-12 16:15:14 +00:00
fix: better tool persistance
This commit is contained in:
1 parent
96b423795a
commit
ce0402a003
3 files changed
+114
-6
No files matched your search
@@ -250,8 +250,11 @@ class BaseAnswerResource:
|
||||
# continuation state is committed (or the stateless finalize path is
|
||||
# reached): the v1 translator turns it into ``finish_reason:"tool_calls"``,
|
||||
# and a client that resumes on that signal would otherwise race
|
||||
# ``save_state``, miss the pending state, and fall back to a stateless
|
||||
# ``persist=False`` round that never persists the final answer.
|
||||
# ``save_state``, miss the pending state, and fall back to the
|
||||
# transcript-rebuild round. That fallback still persists the final
|
||||
# answer when a conversation is mapped, but only as an appended
|
||||
# empty-prompt turn — it cannot finalize the reserved WAL row, which
|
||||
# would be stranded non-terminal for the reconciler.
|
||||
pending_pause_event: Optional[dict] = None
|
||||
|
||||
# One id shared across the WAL row, primary LLM (token_usage
|
||||
|
||||
@@ -298,9 +298,18 @@ def chat_completions():
|
||||
internal_data["tool_actions"],
|
||||
)
|
||||
# A missing/expired durable continuation has no reserved WAL
|
||||
# row to finalize. Run it statelessly without appending a blank
|
||||
# sibling turn to the mapped conversation.
|
||||
internal_data["persist"] = False
|
||||
# row to finalize, so nothing mid-loop is persisted — but the
|
||||
# FINAL turn of this round must still land in the mapped
|
||||
# conversation, else a loop whose first resume produces the
|
||||
# answer (one pause, then stop — the common single-search
|
||||
# shape) persists only the empty tool-call row and the answer
|
||||
# is lost upstream. With persist on, the finished answer is
|
||||
# APPENDED as a new empty-prompt turn (there is no reserved
|
||||
# row to finalize into); that's the accepted shape here. No
|
||||
# worthless rows appear along the way: continuation rounds
|
||||
# reserve no WAL row and the pause path never appends. Only a
|
||||
# round with no mapped conversation at all stays stateless.
|
||||
internal_data["persist"] = bool(internal_data.get("conversation_id"))
|
||||
continuation = {
|
||||
"messages": messages,
|
||||
"tools_dict": tools_dict,
|
||||
|
||||
@@ -711,9 +711,105 @@ class TestV1ContinuationRoutesStatelessly:
|
||||
assert continuation["reserved_message_id"] is None
|
||||
assert continuation["request_id"] is None
|
||||
assert captured.get("finalize_tool_pause_as_complete") is False
|
||||
# Missing durable state runs without appending a blank sibling turn.
|
||||
# A mapped conversation keeps persistence on: the final turn of the
|
||||
# round must land in it (as an appended answer turn — there is no
|
||||
# reserved WAL row to finalize), else the answer is silently lost.
|
||||
assert captured.get("conversation_id") == "conv-1"
|
||||
assert captured.get("should_persist") is True
|
||||
|
||||
def test_continuation_without_conversation_id_stays_stateless(self):
|
||||
"""No mapped conversation → the round runs with persistence off."""
|
||||
app = self._build_app()
|
||||
|
||||
def _fake_translate(data, api_key):
|
||||
# Translator behavior for a stateless continuation (opencode
|
||||
# et al.): no conversation_id, persist defaults to False.
|
||||
return {
|
||||
"question": "",
|
||||
"tool_actions": [{"call_id": "c1", "result": "r"}],
|
||||
"persist": False,
|
||||
"messages": data["messages"],
|
||||
}
|
||||
|
||||
fake_processor = MagicMock()
|
||||
fake_processor.decoded_token = {"sub": "owner"}
|
||||
fake_processor.conversation_id = None
|
||||
fake_processor.agent_config = {"user_api_key": "k"}
|
||||
fake_processor.agent_id = None
|
||||
fake_processor.model_id = "m"
|
||||
fake_processor.model_user_id = None
|
||||
fake_processor.reserved_message_id = None
|
||||
fake_processor.request_id = None
|
||||
fake_processor.build_continuation_from_messages.return_value = (
|
||||
MagicMock(), [], {}, [{"call_id": "c1"}], [{"call_id": "c1", "result": "r"}], ""
|
||||
)
|
||||
|
||||
captured: Dict[str, Any] = {}
|
||||
|
||||
def _capture_complete_stream(**kw):
|
||||
captured.update(kw)
|
||||
return iter(['data: {"type": "end"}'])
|
||||
|
||||
fake_helper = MagicMock()
|
||||
fake_helper.check_usage.return_value = None
|
||||
fake_helper.complete_stream.side_effect = _capture_complete_stream
|
||||
fake_helper.process_response_stream.return_value = {
|
||||
"error": None,
|
||||
"conversation_id": None,
|
||||
"answer": "done",
|
||||
"sources": [],
|
||||
"tool_calls": [],
|
||||
"thought": "",
|
||||
}
|
||||
|
||||
@contextmanager
|
||||
def _yield_conn():
|
||||
yield MagicMock()
|
||||
|
||||
with patch(
|
||||
"application.api.v1.routes._lookup_agent",
|
||||
return_value={"id": "agent-1", "name": "Agent", "user_id": "owner"},
|
||||
), patch(
|
||||
"application.api.v1.routes.translate_request",
|
||||
side_effect=_fake_translate,
|
||||
), patch(
|
||||
"application.api.v1.routes.StreamProcessor",
|
||||
return_value=fake_processor,
|
||||
), patch(
|
||||
"application.api.v1.routes._V1AnswerHelper",
|
||||
return_value=fake_helper,
|
||||
), patch(
|
||||
"application.api.v1.routes.db_readonly",
|
||||
_yield_conn,
|
||||
), patch(
|
||||
"application.api.v1.routes.translate_response",
|
||||
return_value={"id": "x", "choices": []},
|
||||
):
|
||||
with app.test_client() as c:
|
||||
resp = c.post(
|
||||
"/v1/chat/completions",
|
||||
headers={"Authorization": "Bearer x"},
|
||||
json={
|
||||
"messages": [
|
||||
{"role": "user", "content": "hi"},
|
||||
{
|
||||
"role": "assistant",
|
||||
"tool_calls": [{"id": "c1", "function": {}}],
|
||||
},
|
||||
{"role": "tool", "tool_call_id": "c1", "content": "r"},
|
||||
],
|
||||
},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
fake_processor.build_continuation_from_messages.assert_called_once()
|
||||
fake_processor.resume_from_tool_actions.assert_not_called()
|
||||
# No mapped conversation: nothing to land the answer in, so the
|
||||
# round stays fully stateless (no orphan empty-question rows), and
|
||||
# a further pause finalizes as complete instead of writing state.
|
||||
assert captured.get("conversation_id") is None
|
||||
assert captured.get("should_persist") is False
|
||||
assert captured.get("finalize_tool_pause_as_complete") is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in new issue
Block a user