mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-15 00:16:46 +00:00
Default retrieval to 6 chunks instead of 2
chunks is a total per request, split across the attached sources, so an agent with two sources and the default of 2 got a single chunk from each, and its answers changed with whichever chunk won. 6 gives three per source for about 4-5k more input tokens per retrieval turn. Every literal default moves from 2 to 6: the request default, the retrievers, the internal search tool, workflow agent nodes, scheduled and headless runs, agent create/update/import, the source retrieval config and the frontend forms. Existing agents and sources keep what they store; a source saved with chunks=2 now counts as configured at 2, which is pinned by a test. Headless runs also read chunks=0 as unset (`or 2`), so an agent with retrieval switched off retrieved anyway on scheduled runs; 0 now stays 0.
This commit is contained in:
1 parent
1c19ed8ce2
commit
ab3dbfa1be
33 files changed
+187
-46
No files matched your search
@@ -0,0 +1,68 @@
|
||||
"""``run_agent_headless`` passes the agent's ``chunks`` to the retriever.
|
||||
|
||||
``chunks=0`` switches retrieval off, but ``int(... or 2)`` read it as unset,
|
||||
so a scheduled run of an agent with retrieval off retrieved anyway.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
def _retriever_chunks(agent_config, monkeypatch):
|
||||
"""The ``chunks`` a headless run hands to the retriever."""
|
||||
from docsgpt.agents import headless_runner as hr
|
||||
|
||||
agent = MagicMock(name="agent")
|
||||
agent.gen.return_value = iter([{"answer": "ok"}])
|
||||
agent.llm.token_usage = {"prompt_tokens": 1, "generated_tokens": 1}
|
||||
|
||||
retriever = MagicMock(name="retriever")
|
||||
retriever.search.return_value = []
|
||||
created = {}
|
||||
|
||||
def create_retriever(cls, *args, **kwargs):
|
||||
created.update(kwargs)
|
||||
return retriever
|
||||
|
||||
tool_executor = MagicMock(name="tool_executor")
|
||||
tool_executor.headless_denials = []
|
||||
|
||||
monkeypatch.setattr(hr, "get_prompt", lambda _pid: "system prompt")
|
||||
monkeypatch.setattr(
|
||||
hr.RetrieverCreator, "create_retriever", classmethod(create_retriever),
|
||||
)
|
||||
monkeypatch.setattr(hr, "ToolExecutor", lambda *a, **kw: tool_executor)
|
||||
monkeypatch.setattr(
|
||||
hr.AgentCreator, "create_agent",
|
||||
classmethod(lambda cls, *a, **kw: agent),
|
||||
)
|
||||
|
||||
config = {"user_id": "u1", "id": "agent-1", "default_model_id": "m", **agent_config}
|
||||
with patch("docsgpt.core.model_utils.validate_model_id", return_value=True), \
|
||||
patch("docsgpt.core.model_utils.get_default_model_id", return_value="m"), \
|
||||
patch(
|
||||
"docsgpt.core.model_utils.get_provider_from_model_id",
|
||||
return_value="openai",
|
||||
), \
|
||||
patch("docsgpt.core.model_utils.get_api_key_for_provider", return_value="k"), \
|
||||
patch("docsgpt.utils.calculate_doc_token_budget", return_value=1000):
|
||||
hr.run_agent_headless(config, "do the thing")
|
||||
return created["chunks"]
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestHeadlessRunnerChunks:
|
||||
def test_unset_chunks_uses_the_default(self, monkeypatch):
|
||||
assert _retriever_chunks({}, monkeypatch) == 6
|
||||
|
||||
def test_null_chunks_uses_the_default(self, monkeypatch):
|
||||
assert _retriever_chunks({"chunks": None}, monkeypatch) == 6
|
||||
|
||||
def test_zero_chunks_keeps_retrieval_off(self, monkeypatch):
|
||||
assert _retriever_chunks({"chunks": 0}, monkeypatch) == 0
|
||||
|
||||
def test_explicit_chunks_is_kept(self, monkeypatch):
|
||||
assert _retriever_chunks({"chunks": 4}, monkeypatch) == 4
|
||||
@@ -108,7 +108,7 @@ class TestAgentNodeConfig:
|
||||
assert c.stream_to_user is True
|
||||
assert c.tools == []
|
||||
assert c.sources == []
|
||||
assert c.chunks == "2"
|
||||
assert c.chunks == "6"
|
||||
assert c.retriever == ""
|
||||
assert c.model_id is None
|
||||
assert c.json_schema is None
|
||||
|
||||
@@ -383,7 +383,7 @@ class TestBuildHelpers:
|
||||
def test_build_config_defaults(self):
|
||||
config = build_internal_tool_config(source={"active_docs": ["abc"]})
|
||||
assert config["retriever_name"] == "classic"
|
||||
assert config["chunks"] == 2
|
||||
assert config["chunks"] == 6
|
||||
assert config["doc_token_limit"] == 50000
|
||||
|
||||
def test_internal_tool_id(self):
|
||||
|
||||
@@ -473,7 +473,7 @@ class TestConfigureRetriever:
|
||||
sp = StreamProcessor({}, {"sub": "u"})
|
||||
sp._configure_retriever()
|
||||
assert sp.retriever_config["retriever_name"] == "classic"
|
||||
assert sp.retriever_config["chunks"] == 2
|
||||
assert sp.retriever_config["chunks"] == 6
|
||||
|
||||
def test_agent_overrides(self):
|
||||
from docsgpt.api.answer.services.stream_processor import (
|
||||
@@ -519,7 +519,7 @@ class TestConfigureRetriever:
|
||||
sp._agent_data = {}
|
||||
sp._configure_retriever()
|
||||
assert sp.retriever_config["retriever_name"] == "classic"
|
||||
assert sp.retriever_config["chunks"] == 2
|
||||
assert sp.retriever_config["chunks"] == 6
|
||||
|
||||
def test_invalid_agent_chunks_falls_back(self):
|
||||
from docsgpt.api.answer.services.stream_processor import (
|
||||
@@ -528,7 +528,7 @@ class TestConfigureRetriever:
|
||||
sp = StreamProcessor({}, {"sub": "u"})
|
||||
sp._agent_data = {"chunks": "not-a-number"}
|
||||
sp._configure_retriever()
|
||||
assert sp.retriever_config["chunks"] == 2
|
||||
assert sp.retriever_config["chunks"] == 6
|
||||
|
||||
def test_invalid_request_chunks_falls_back(self):
|
||||
from docsgpt.api.answer.services.stream_processor import (
|
||||
@@ -536,7 +536,7 @@ class TestConfigureRetriever:
|
||||
)
|
||||
sp = StreamProcessor({"chunks": "abc"}, {"sub": "u"})
|
||||
sp._configure_retriever()
|
||||
assert sp.retriever_config["chunks"] == 2
|
||||
assert sp.retriever_config["chunks"] == 6
|
||||
|
||||
def test_isnonedoc_without_api_key_sets_chunks_to_0(self):
|
||||
from docsgpt.api.answer.services.stream_processor import (
|
||||
|
||||
@@ -126,6 +126,15 @@ class TestChunksPrecedence:
|
||||
sp._configure_retriever()
|
||||
return sp.retriever_config["chunks"]
|
||||
|
||||
def test_default_applies_when_nothing_sets_chunks(self):
|
||||
assert self._sp() == 6
|
||||
|
||||
def test_source_stored_at_the_old_default_counts_as_configured(self):
|
||||
"""Accepted when the default moved from 2 to 6: sources created while
|
||||
the UI always sent the full config store ``chunks=2`` explicitly, and
|
||||
stay a per-source setting of 2 rather than being migrated."""
|
||||
assert self._sp(request_chunks="7", source_chunks=2) == 2
|
||||
|
||||
def test_request_applies_when_source_is_unconfigured(self):
|
||||
assert self._sp(request_chunks="7") == 7
|
||||
|
||||
@@ -139,7 +148,7 @@ class TestChunksPrecedence:
|
||||
("-5", 0),
|
||||
("100000", 500),
|
||||
("501", 500),
|
||||
("abc", 2),
|
||||
("abc", 6),
|
||||
],
|
||||
)
|
||||
def test_request_chunks_is_clamped(self, sent, expected):
|
||||
|
||||
@@ -1202,7 +1202,7 @@ class TestConfigureAgent:
|
||||
assert sp.retriever_config["chunks"] == 5
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_configure_agent_invalid_chunks_defaults_to_2(self):
|
||||
def test_configure_agent_invalid_chunks_uses_the_default(self):
|
||||
sp = self._make_sp()
|
||||
sp._resolve_agent_id = MagicMock(return_value="agent_id_1")
|
||||
sp._get_agent_key = MagicMock(return_value=("agent_key", False, None))
|
||||
@@ -1220,7 +1220,7 @@ class TestConfigureAgent:
|
||||
sp._configure_agent()
|
||||
sp.model_id = "test-model"
|
||||
sp._configure_retriever()
|
||||
assert sp.retriever_config["chunks"] == 2
|
||||
assert sp.retriever_config["chunks"] == 6
|
||||
|
||||
|
||||
# ---- Additional coverage: _load_conversation_history ----
|
||||
|
||||
@@ -176,6 +176,20 @@ def test_import_by_slug_idempotent(pg_conn):
|
||||
assert len(AgentsRepository(pg_conn).list_for_user(user)) == 1
|
||||
|
||||
|
||||
def test_import_without_chunks_uses_the_default(pg_conn):
|
||||
user = "u_default_chunks"
|
||||
result = apply_import(pg_conn, user, _doc(name="Plain Bot", _slug="plain-bot"))
|
||||
agent = AgentsRepository(pg_conn).get(result["agent_id"], user)
|
||||
assert agent["chunks"] == 6
|
||||
|
||||
|
||||
def test_import_keeps_explicit_chunks(pg_conn):
|
||||
user = "u_explicit_chunks"
|
||||
result = apply_import(pg_conn, user, _doc(name="Off Bot", _slug="off-bot", chunks=0))
|
||||
agent = AgentsRepository(pg_conn).get(result["agent_id"], user)
|
||||
assert agent["chunks"] == 0
|
||||
|
||||
|
||||
def test_import_missing_source_drafts_and_warns(pg_conn):
|
||||
user = "u_missing"
|
||||
doc = _doc(
|
||||
|
||||
@@ -426,6 +426,31 @@ class TestCreateAgent:
|
||||
agents = AgentsRepository(pg_conn).list_for_user(user)
|
||||
assert any(a["name"] == "My Draft Agent" for a in agents)
|
||||
|
||||
def test_agent_created_without_chunks_stores_the_default(self, app, pg_conn):
|
||||
# With two sources, 2 left each one a single chunk, so the answer hung
|
||||
# on which chunk won; 6 is the default now.
|
||||
from docsgpt.api.user.agents.routes import CreateAgent
|
||||
from docsgpt.storage.db.repositories.agents import AgentsRepository
|
||||
|
||||
user = "u-create-default-chunks"
|
||||
|
||||
with _patch_db(pg_conn), app.test_request_context(
|
||||
"/api/create_agent",
|
||||
method="POST",
|
||||
json={
|
||||
"name": "Handbook Bot",
|
||||
"description": "d",
|
||||
"agent_type": "classic",
|
||||
"status": "draft",
|
||||
},
|
||||
):
|
||||
from flask import request
|
||||
request.decoded_token = {"sub": user}
|
||||
response = CreateAgent().post()
|
||||
assert response.status_code == 201
|
||||
(agent,) = AgentsRepository(pg_conn).list_for_user(user)
|
||||
assert agent["chunks"] == 6
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# UpdateAgent — big method with many validation branches
|
||||
|
||||
@@ -40,7 +40,7 @@ class TestParseLenient:
|
||||
r = RetrievalConfig()
|
||||
assert r.retriever == "classic"
|
||||
assert r.exposure == "prefetch"
|
||||
assert r.chunks == 2
|
||||
assert r.chunks == 6
|
||||
assert r.score_threshold is None
|
||||
assert r.rephrase_query is True
|
||||
assert r.reranker is None
|
||||
|
||||
@@ -74,6 +74,22 @@ class TestDispatcherGrouping:
|
||||
assert "b" not in retrievals
|
||||
|
||||
|
||||
def test_source_stored_at_the_old_default_is_an_override(self, _patch_llm_creator):
|
||||
# Accepted when the default moved from 2 to 6: sources saved with the
|
||||
# full config store chunks=2 and keep it as their own setting, while a
|
||||
# new upload stores 6, the default, and takes the global path.
|
||||
sources = [
|
||||
{"id": "old", "retrieval": RetrievalConfig(chunks=2)},
|
||||
{"id": "new", "retrieval": RetrievalConfig(chunks=6)},
|
||||
]
|
||||
d = Dispatcher(source={"question": "q", "active_docs": ["old", "new"]}, sources=sources)
|
||||
retrievals = d._groups[0]["retrievals"]
|
||||
assert retrievals["old"].chunks == 2
|
||||
assert "new" not in retrievals
|
||||
|
||||
def test_default_budget_is_six(self, _patch_llm_creator):
|
||||
assert Dispatcher(source={"question": "q", "active_docs": ["a"]}).chunks == 6
|
||||
|
||||
def test_graph_options_count_as_an_override(self, _patch_llm_creator):
|
||||
"""A graph source that changes only its graph options still needs its
|
||||
config carried over: those options live on the per-source retrieval the
|
||||
|
||||
@@ -151,7 +151,7 @@ class TestClassicRAGInit:
|
||||
|
||||
def test_chunks_invalid_string_defaults(self, _patch_llm_creator):
|
||||
rag = _make_rag(chunks="abc")
|
||||
assert rag.chunks == 2
|
||||
assert rag.chunks == 6
|
||||
|
||||
def test_decoded_token_none(self, _patch_llm_creator):
|
||||
rag = _make_rag(decoded_token=None)
|
||||
|
||||
Reference in new issue
Block a user