refactor(tools): extract document tools to handle registry (#3666)

* feat(tools): add document management tool handlers to the agent_tools module

* feat(tools): extraced document tools for create, update, edit, suggest, and manage from tool_implementations.py

* feat(tests): refactor document tool tests to use TOOL_HANDLERS and document_tools

* refactor(tools): add document tool dispatcher and updated tool calling path

* refactor(tools): remove duplicated document management functions

* refactor(tools): removing unused functions and adding new import paths

* refactor(tools): update document tool execute methods to use context dictionary

* refactor(tests): update import paths for document tools in test files

* refactor(tests): update owner parameter format in document management tests

* refactor(tests): update import path for _owned_document_query

* feat(tools): add document management tool handlers to the agent_tools module

* feat(tools): extraced document tools for create, update, edit, suggest, and manage from tool_implementations.py

* feat(tests): refactor document tool tests to use TOOL_HANDLERS and document_tools

* refactor(tools): add document tool dispatcher and updated tool calling path

* refactor(tools): remove duplicated document management functions

* refactor(tools): removing unused functions and adding new import paths

* refactor(tools): update document tool execute methods to use context dictionary

* refactor(tests): update import paths for document tools in test files

* refactor(tests): update owner parameter format in document management tests

* refactor(tests): update import path for _owned_document_query

* refactor: update import paths for document tools

* fix(tests): correct source path for document ID test
This commit is contained in:
Yeoh Ing Ji
2026-06-10 09:41:52 +01:00
committed by GitHub
parent fc8e6366dd
commit 3e49658204
12 changed files with 724 additions and 661 deletions
+2 -3
View File
@@ -6,13 +6,12 @@ injection re-surfaced the closed doc in later, unrelated chats. The document
routes now call clear_active_document() on detach/delete; this pins that helper.
"""
from src.tool_implementations import (
from src.agent_tools.document_tools import (
set_active_document,
get_active_document,
clear_active_document,
clear_active_document
)
def test_clear_matching_id_resets_pointer():
set_active_document("doc-123")
assert get_active_document() == "doc-123"
@@ -30,7 +30,7 @@ import routes.document_routes as droutes
from core.database import Document
from core.database import Session as DbSession
from routes.document_helpers import DocumentPatch
from src.tool_implementations import set_active_document, get_active_document
from src.agent_tools.document_tools import set_active_document, get_active_document
_TMPDB = tempfile.NamedTemporaryFile(suffix=".db", delete=False)
_ENGINE = create_engine(
+1 -1
View File
@@ -13,7 +13,7 @@ _REPO = Path(__file__).resolve().parents[1]
def test_chat_document_links_use_the_document_id():
"""The list/open tool must anchor to the real document id, not a slug —
a slug 404s against the UUID-keyed /api/document/<id> route."""
src = (_REPO / "src" / "tool_implementations.py").read_text(encoding="utf-8")
src = (_REPO / "src" / "agent_tools" /"document_tools.py").read_text(encoding="utf-8")
assert "(#document-{d.id})" in src
assert "(#document-{doc.id})" in src
+33 -18
View File
@@ -2,7 +2,11 @@ import asyncio
import sys
import types
from src import tool_implementations as tools
from src.agent_tools import TOOL_HANDLERS
from src.agent_tools.document_tools import (
_owned_document_query,
set_active_document,
)
class _Column:
@@ -76,14 +80,14 @@ def _install_database_stub(monkeypatch, module_name, query):
def test_owned_document_query_rejects_missing_owner():
query = _Query()
assert tools._owned_document_query(query, _Document, None) is query
assert _owned_document_query(query, _Document, None) is query
assert False in query.filters
def test_owned_document_query_filters_to_owner():
query = _Query()
assert tools._owned_document_query(query, _Document, "alice") is query
assert _owned_document_query(query, _Document, "alice") is query
assert ("owner", "eq", "alice") in query.filters
@@ -91,7 +95,9 @@ def test_manage_documents_list_filters_to_calling_owner(monkeypatch):
query = _Query()
_install_database_stub(monkeypatch, "core.database", query)
result = asyncio.run(tools.do_manage_documents('{"action":"list"}', owner="alice"))
result = asyncio.run(
TOOL_HANDLERS["manage_documents"]('{"action":"list"}', {"owner": "alice"})
)
assert result["documents"] == []
assert ("owner", "eq", "alice") in query.filters
@@ -102,7 +108,9 @@ def test_manage_documents_read_filters_to_calling_owner(monkeypatch):
_install_database_stub(monkeypatch, "core.database", query)
result = asyncio.run(
tools.do_manage_documents('{"action":"read","document_id":"doc-bob"}', owner="alice")
TOOL_HANDLERS["manage_documents"](
'{"action":"read","document_id":"doc-bob"}', {"owner": "alice"}
)
)
assert result["exit_code"] == 1
@@ -113,11 +121,13 @@ def test_manage_documents_read_filters_to_calling_owner(monkeypatch):
def test_update_document_active_id_filters_to_calling_owner(monkeypatch):
query = _Query()
_install_database_stub(monkeypatch, "src.database", query)
tools.set_active_document("doc-bob")
set_active_document("doc-bob")
try:
result = asyncio.run(tools.do_update_document("new content", owner="alice"))
result = asyncio.run(
TOOL_HANDLERS["update_document"]("new content", {"owner": "alice"})
)
finally:
tools.set_active_document(None)
set_active_document(None)
assert result["error"] == "No documents exist to update"
assert ("id", "eq", "doc-bob") in query.filters
@@ -127,14 +137,16 @@ def test_update_document_active_id_filters_to_calling_owner(monkeypatch):
def test_suggest_document_active_id_filters_to_calling_owner(monkeypatch):
query = _Query()
_install_database_stub(monkeypatch, "src.database", query)
tools.set_active_document("doc-bob")
set_active_document("doc-bob")
try:
result = asyncio.run(tools.do_suggest_document(
"<<<FIND>>>\nold\n<<<SUGGEST>>>\nnew\n<<<REASON>>>\nbetter\n<<<END>>>",
owner="alice",
))
result = asyncio.run(
TOOL_HANDLERS["suggest_document"](
"<<<FIND>>>\nold\n<<<SUGGEST>>>\nnew\n<<<REASON>>>\nbetter\n<<<END>>>",
{"owner": "alice"},
)
)
finally:
tools.set_active_document(None)
set_active_document(None)
assert result["error"] == "Document doc-bob not found"
assert ("id", "eq", "doc-bob") in query.filters
@@ -144,7 +156,10 @@ def test_suggest_document_active_id_filters_to_calling_owner(monkeypatch):
def test_document_tool_dispatch_forwards_owner():
source = open("src/tool_execution.py", encoding="utf-8").read()
assert "do_create_document(content, session_id=session_id, owner=owner)" in source
assert "do_update_document(content, owner=owner)" in source
assert "do_edit_document(content, owner=owner)" in source
assert "do_suggest_document(content, owner=owner)" in source
assert "_document_tool_dispatch(tool, content, session_id, owner)" in source
# Also verify TOOL_HANDLERS has the expected entries
for key in ("create_document", "update_document", "edit_document",
"suggest_document", "manage_documents"):
assert key in TOOL_HANDLERS, f"TOOL_HANDLERS missing key: {key}"
assert callable(TOOL_HANDLERS[key]), f"TOOL_HANDLERS[{key!r}] is not callable"
+1 -1
View File
@@ -1,5 +1,5 @@
"""Tests for _owned_document_query owner scoping (src/tool_implementations.py)."""
from src.tool_implementations import _owned_document_query
from src.agent_tools.document_tools import _owned_document_query
class _FakeQuery: