From ae9926f54d4452eea1f6ba8cf122105bb7588c2d Mon Sep 17 00:00:00 2001 From: phernandez Date: Sat, 18 Jan 2025 10:23:01 -0600 Subject: [PATCH] add discussion tools --- src/basic_memory/api/routers/memory_router.py | 4 +- src/basic_memory/mcp/tools/__init__.py | 4 + src/basic_memory/mcp/tools/discussion.py | 105 ++++++++++++++++++ src/basic_memory/schemas/memory.py | 72 ++++-------- src/basic_memory/services/context_service.py | 9 +- tests/api/test_memory_router.py | 2 +- tests/conftest.py | 2 +- tests/mcp/test_tool_discussion.py | 66 +++++++++++ tests/schemas/test_memory_url.py | 49 +++----- tests/services/test_context_service.py | 16 ++- tests/services/test_search_service.py | 11 ++ 11 files changed, 235 insertions(+), 105 deletions(-) create mode 100644 src/basic_memory/mcp/tools/discussion.py create mode 100644 tests/mcp/test_tool_discussion.py diff --git a/src/basic_memory/api/routers/memory_router.py b/src/basic_memory/api/routers/memory_router.py index 0430d3b6..6b718f9c 100644 --- a/src/basic_memory/api/routers/memory_router.py +++ b/src/basic_memory/api/routers/memory_router.py @@ -40,13 +40,13 @@ async def get_memory_context( """Get rich context from memory:// URI.""" # add the project name from the config to the url as the "host # Parse URI - memory_url = MemoryUrl.parse(f"memory://{config.project}/{uri}") + memory_url = MemoryUrl(f"memory://{config.project}/{uri}") # Parse timeframe since = parse_timeframe(timeframe) # Build context - context = await context_service.build_context(str(memory_url), depth=depth, since=since) + context = await context_service.build_context(memory_url, depth=depth, since=since) primary_entities = [SearchResult(**asdict(r)) for r in context["primary_entities"]] related_entities = [RelatedResult(**asdict(r)) for r in context["related_entities"]] diff --git a/src/basic_memory/mcp/tools/__init__.py b/src/basic_memory/mcp/tools/__init__.py index 9b19cb43..3dfb3b7f 100644 --- a/src/basic_memory/mcp/tools/__init__.py +++ b/src/basic_memory/mcp/tools/__init__.py @@ -10,6 +10,7 @@ from basic_memory.mcp.tools import knowledge # noqa: F401 from basic_memory.mcp.tools import search # noqa: F401 from basic_memory.mcp.tools import discovery # noqa: F401 from basic_memory.mcp.tools import activity # noqa: F401 +from basic_memory.mcp.tools.discussion import get_discussion_context # Export the tools from basic_memory.mcp.tools.knowledge import ( @@ -55,4 +56,7 @@ __all__ = [ # Activity tools "get_recent_activity", + + # memory tools + "get_discussion_context" ] \ No newline at end of file diff --git a/src/basic_memory/mcp/tools/discussion.py b/src/basic_memory/mcp/tools/discussion.py new file mode 100644 index 00000000..dfada2b4 --- /dev/null +++ b/src/basic_memory/mcp/tools/discussion.py @@ -0,0 +1,105 @@ +"""Discussion context tools for Basic Memory MCP server.""" + +from typing import Optional + +from basic_memory.mcp.async_client import client +from basic_memory.mcp.server import mcp +from basic_memory.schemas.memory import GraphContext, MemoryUrl + + +@mcp.tool( + category="discussion", + description="Get discussion context from a memory:// URI to continue conversations naturally.", + examples=[ + { + "name": "Continue Previous Discussion", + "description": "Load context to continue a technical discussion", + "code": """ +# Get context for previous discussion about search +context = await get_discussion_context( + url="memory://specs/search-refactor", + depth=2, + timeframe="7d" +) + +# Access the context components +primary = context.primary_entities # Main discussion topic +related = context.related_entities # Related concepts +meta = context.metadata # Discussion metadata + +# Analyze primary discussion topics +print("\\nPrimary Discussion Topic:") +for entity in primary: + print(f"- {entity.title}") + print(f" Type: {entity.type}") + if "status" in entity.metadata: + print(f" Status: {entity.metadata['status']}") + +# Analyze related content +print("\\nRelated Topics:") +for item in related: + print(f"- {item.title}") + if item.relation_type: + print(f" Relation: {item.relation_type}") +""" + }, + { + "name": "Pattern Matching and Time Filtering", + "description": "Search for matching discussions within a timeframe", + "code": """ +# Find recent discussions matching a pattern +context = await get_discussion_context( + url="memory://design/*", # Match all design documents + depth=1, # Direct relations only + timeframe="3d" # Last 3 days only +) + +print(f"Found {context.metadata['matched_entities']} matching discussions") +print(f"Total related items: {context.metadata['total_entities']}") + +# Group by document type +by_type = {} +for entity in context.primary_entities: + doc_type = entity.type + if doc_type not in by_type: + by_type[doc_type] = [] + by_type[doc_type].append(entity) + +# Show summary by type +for doc_type, entities in by_type.items(): + print(f"\\n{doc_type.title()}:") + for entity in entities: + print(f"- {entity.title}") + print(f" Added: {entity.metadata.get('created_at', 'Unknown')}") +""" + } + ], +) +async def get_discussion_context( + url: MemoryUrl, + depth: Optional[int] = 2, + timeframe: Optional[str] = "7d", +) -> GraphContext: + """Get context needed to continue a discussion. + + This tool enables natural continuation of discussions by loading relevant context + from memory:// URIs. It uses pattern matching to find relevant content and builds + a rich context graph of related information. + + Args: + url: memory:// URI pointing to discussion content (e.g. memory://specs/search) + depth: How many relation hops to traverse (default: 2) + timeframe: How far back to look, e.g. "7d", "24h" (default: "7d") + + Returns: + GraphContext containing: + - primary_entities: Directly matched content + - related_entities: Connected content via relations + - metadata: Context building info + """ + # Map directly to the memory endpoint + memory_url = MemoryUrl.validate(url) + response = await client.get( + f"/memory/{memory_url.relative_path()}", params={"depth": depth, "timeframe": timeframe} + ) + return GraphContext.model_validate(response.json()) \ No newline at end of file diff --git a/src/basic_memory/schemas/memory.py b/src/basic_memory/schemas/memory.py index 26cde886..07e86031 100644 --- a/src/basic_memory/schemas/memory.py +++ b/src/basic_memory/schemas/memory.py @@ -1,79 +1,45 @@ """Schemas for memory context.""" from typing import Dict, List, Optional, Any - -from pydantic import BaseModel, Field -from pydantic import field_validator +from pydantic import AnyUrl, Field, BaseModel from basic_memory.schemas.search import SearchResult, RelatedResult +from basic_memory.config import config """Memory URL schema for knowledge addressing. -The memory:// URL scheme provides a unified way to address knowledge across projects: -memory://project-name/path/to/content +The memory:// URL scheme provides a unified way to address knowledge: Examples: - memory://basic-memory/specs/search/* # Pattern matching - memory://basic-memory/specs/xyz # Exact permalink - memory://basic-memory/related/sync # Related content + memory://specs/search/* # Pattern matching + memory://specs/xyz # direct reference """ -class MemoryUrl(BaseModel): +class MemoryUrl(AnyUrl): """memory:// URL scheme for knowledge addressing.""" - - scheme: str = Field(default="memory", frozen=True) - host: str # Project identifier - path: str # Full path + + allowed_schemes = {'memory'} # Query params params: Dict[str, Any] = Field(default_factory=dict) # For special modes like 'related' - @field_validator("scheme") @classmethod - def validate_scheme(cls, v: str) -> str: - """Validate URL scheme.""" - if v != "memory": - raise ValueError("URL must use memory:// scheme") - return v + def validate(cls, url: str) -> "MemoryUrl": + """Validate and construct a MemoryUrl.""" - @field_validator("host") - @classmethod - def validate_host(cls, v: Optional[str]) -> str: - """Validate host (project identifier).""" - if not v: - raise ValueError("URL must include project/context identifier") - return v + memory_url = cls(url) + + # if the url host value is not the project name, assume the default project + if memory_url.host != config.project: + memory_url = cls(f"memory://{config.project}/{memory_url.host}{memory_url.path}") - @classmethod - def parse(cls, url_str: str) -> "MemoryUrl": - """Parse a memory:// URL string.""" - # Split scheme and rest - if "://" not in url_str: - raise ValueError("URL must include scheme (memory://)") - - scheme, rest = url_str.split("://", 1) - - # Split host and path - parts = rest.split("/", 1) - if len(parts) != 2: - raise ValueError("URL must include both host and path") - - host, path = parts - path = "/" + path # Add leading slash - - # Create base URL - url = cls(scheme=scheme, host=host, path=path) - return url - - @property - def project(self) -> str: - """Get the project/context identifier.""" - return self.host + return memory_url def relative_path(self) -> str: """Get the path without leading slash.""" - return self.path[1:] if self.path.startswith("/") else self.path + path = self.path + return path[1:] if path.startswith("/") else path def __str__(self) -> str: """Convert back to URL string.""" @@ -101,4 +67,4 @@ class GraphContext(BaseModel): "total_entities": 8, "total_relations": 12, }, - ) + ) \ No newline at end of file diff --git a/src/basic_memory/services/context_service.py b/src/basic_memory/services/context_service.py index 20c60129..4b627c03 100644 --- a/src/basic_memory/services/context_service.py +++ b/src/basic_memory/services/context_service.py @@ -49,15 +49,12 @@ class ContextService: async def build_context( self, - uri: str, + memory_url: MemoryUrl, depth: int = 2, since: Optional[datetime] = None, ): """Build rich context from a memory:// URI.""" - logger.debug(f"Building context for URI {uri}") - - # Parse the URI - memory_url = MemoryUrl.parse(uri) + logger.debug(f"Building context for URI {memory_url}") # Pattern matching - use search if '*' in memory_url.relative_path(): @@ -85,7 +82,7 @@ class ContextService: "primary_entities": primary, "related_entities": related, "metadata": { - "uri": uri, + "uri": memory_url.relative_path(), "depth": depth, "timeframe": since.isoformat() if since else None, "generated_at": datetime.now(timezone.utc).isoformat(), diff --git a/tests/api/test_memory_router.py b/tests/api/test_memory_router.py index 60baca8c..c39b8676 100644 --- a/tests/api/test_memory_router.py +++ b/tests/api/test_memory_router.py @@ -17,7 +17,7 @@ async def test_get_memory_context(client, test_graph): assert len(context.related_entities) > 0 # Verify metadata - assert context.metadata["uri"] == "memory://default/test/root" + assert context.metadata["uri"] == "test/root" assert context.metadata["depth"] == 1 # default depth #assert context.metadata["timeframe"] == "7d" # default timeframe assert isinstance(context.metadata["generated_at"], str) diff --git a/tests/conftest.py b/tests/conftest.py index 931d2f0f..fb6ab7f8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -41,7 +41,7 @@ def anyio_backend(): def test_config(tmp_path) -> ProjectConfig: """Test configuration using in-memory DB.""" config = ProjectConfig( - name="test", + project="test-project", ) config.home = tmp_path diff --git a/tests/mcp/test_tool_discussion.py b/tests/mcp/test_tool_discussion.py new file mode 100644 index 00000000..b89de8de --- /dev/null +++ b/tests/mcp/test_tool_discussion.py @@ -0,0 +1,66 @@ +"""Tests for discussion context MCP tool.""" + +import pytest +from basic_memory.mcp.tools.discussion import get_discussion_context +from basic_memory.schemas.memory import GraphContext + +@pytest.mark.asyncio +async def test_get_basic_discussion_context(client, test_graph): + """Test getting basic discussion context.""" + context = await get_discussion_context( + url="memory://test/root" + ) + + assert isinstance(context, GraphContext) + assert len(context.primary_entities) == 1 + assert context.primary_entities[0].permalink == "test/root" + assert len(context.related_entities) > 0 + + # Verify metadata + assert context.metadata["uri"] == "test/root" + assert context.metadata["depth"] == 2 # default depth + assert context.metadata["timeframe"] is not None + assert isinstance(context.metadata["generated_at"], str) + assert context.metadata["matched_entities"] == 1 + +@pytest.mark.asyncio +async def test_get_discussion_context_pattern(client, test_graph): + """Test getting context with pattern matching.""" + context = await get_discussion_context( + url="memory://test/*", + depth=1 + ) + + assert isinstance(context, GraphContext) + assert len(context.primary_entities) > 1 # Should match multiple test/* paths + assert all("test/" in e.permalink for e in context.primary_entities) + assert context.metadata["depth"] == 1 + +@pytest.mark.asyncio +async def test_get_discussion_context_timeframe(client, test_graph): + """Test timeframe parameter filtering.""" + # Get recent context + recent_context = await get_discussion_context( + url="memory://test/root", + timeframe="1d" # Last 24 hours + ) + + # Get older context + older_context = await get_discussion_context( + url="memory://test/root", + timeframe="30d" # Last 30 days + ) + + assert len(older_context.related_entities) >= len(recent_context.related_entities) + +@pytest.mark.asyncio +async def test_get_discussion_context_not_found(client): + """Test handling of non-existent URIs.""" + context = await get_discussion_context( + url="memory://test/does-not-exist" + ) + + assert isinstance(context, GraphContext) + assert len(context.primary_entities) == 0 + assert len(context.related_entities) == 0 + assert context.metadata["matched_entities"] == 0 diff --git a/tests/schemas/test_memory_url.py b/tests/schemas/test_memory_url.py index 59361e85..43c690cc 100644 --- a/tests/schemas/test_memory_url.py +++ b/tests/schemas/test_memory_url.py @@ -3,72 +3,49 @@ import pytest from pydantic import ValidationError +from basic_memory.config import config from basic_memory.schemas.memory import MemoryUrl def test_basic_permalink(): """Test basic permalink parsing.""" - url = MemoryUrl.parse("memory://basic-memory/specs/search") + url = MemoryUrl.validate(f"memory://{config.project}/specs/search") assert url.scheme == "memory" - assert url.host == "basic-memory" + assert url.host == config.project assert url.path == "/specs/search" - assert url.params == {} def test_glob_pattern(): """Test pattern matching.""" - url = MemoryUrl.parse("memory://basic-memory/specs/search/*") - assert url.host == "basic-memory" + url = MemoryUrl.validate(f"memory://{config.project}/specs/search/*") + assert url.host == config.project assert url.path == "/specs/search/*" def test_related_prefix(): """Test related content prefix.""" - url = MemoryUrl.parse("memory://basic-memory/related/specs/search") - assert url.host == "basic-memory" + url = MemoryUrl.validate(f"memory://{config.project}/related/specs/search") + assert url.host == config.project assert url.path == "/related/specs/search" def test_context_prefix(): """Test context prefix.""" - url = MemoryUrl.parse("memory://basic-memory/context/current") - assert url.host == "basic-memory" + url = MemoryUrl.validate(f"memory://{config.project}/context/current") + assert url.host == config.project assert url.path == "/context/current" -def test_invalid_scheme(): - """Test that other schemes are rejected.""" - with pytest.raises(ValidationError): - MemoryUrl.parse("http://basic-memory/specs/search") - - -def test_missing_host(): - """Test that host is required.""" - with pytest.raises(ValidationError): - MemoryUrl.parse("memory:///specs/search") - def test_complex_pattern(): """Test multiple glob patterns.""" - url = MemoryUrl.parse("memory://basic-memory/specs/*/search/*") - assert url.host == "basic-memory" + url = MemoryUrl.validate(f"memory://{config.project}/specs/*/search/*") + assert url.host == config.project assert url.path == "/specs/*/search/*" -def test_url_reconstruction(): - """Test converting back to string.""" - original = "memory://basic-memory/specs/search" - url = MemoryUrl.parse(original) - assert str(url) == original - - -def test_relative_path(): +def test_path_with_no_host(): """Test getting path without leading slash.""" - url = MemoryUrl.parse("memory://basic-memory/specs/search") + url = MemoryUrl.validate("memory://specs/search") assert url.relative_path() == "specs/search" - -def test_project_property(): - """Test project name access.""" - url = MemoryUrl.parse("memory://basic-memory/specs/search") - assert url.project == "basic-memory" diff --git a/tests/services/test_context_service.py b/tests/services/test_context_service.py index aec175ac..552c5a95 100644 --- a/tests/services/test_context_service.py +++ b/tests/services/test_context_service.py @@ -5,7 +5,9 @@ from datetime import datetime, timedelta, UTC import pytest import pytest_asyncio +from basic_memory.config import config from basic_memory.repository.search_repository import SearchIndexRow +from basic_memory.schemas.memory import MemoryUrl from basic_memory.schemas.search import SearchItemType from basic_memory.services.context_service import ContextService @@ -139,14 +141,14 @@ async def test_find_connected_timeframe(context_service, test_graph, search_repo @pytest.mark.asyncio async def test_build_context(context_service, test_graph): """Test exact permalink lookup.""" - url = "memory://not_used/test/root" + url = MemoryUrl(f"memory://{config.project}/test/root") results = await context_service.build_context(url) matched_entities = results["metadata"]["matched_entities"] primary_entities = results["primary_entities"] related_entities = results["related_entities"] total_entities = results["metadata"]["total_entities"] - assert results["metadata"]["uri"] == url + assert results["metadata"]["uri"] == url.relative_path() assert results["metadata"]["depth"] == 2 assert matched_entities == 1 assert len(primary_entities) == 1 @@ -157,19 +159,21 @@ async def test_build_context(context_service, test_graph): @pytest.mark.asyncio async def test_build_context_pattern(context_service, test_graph): """Test exact permalink lookup.""" - url = "memory://not_used/test/connected*" + url = MemoryUrl("memory://not_used/test/connected*") results = await context_service.build_context(url) matched_entities = results["metadata"]["matched_entities"] primary_entities = results["primary_entities"] related_entities = results["related_entities"] total_entities = results["metadata"]["total_entities"] + + #TODO assert pattern found @pytest.mark.asyncio async def test_build_context_not_found(context_service): """Test handling non-existent permalinks.""" - context = await context_service.build_context("memory://project/does/not/exist") + context = await context_service.build_context(MemoryUrl("memory://project/does/not/exist")) assert len(context["primary_entities"]) == 0 assert len(context["related_entities"]) == 0 @@ -177,9 +181,9 @@ async def test_build_context_not_found(context_service): @pytest.mark.asyncio async def test_context_metadata(context_service, test_graph): """Test metadata is correctly populated.""" - context = await context_service.build_context("memory://project/test/root", depth=2) + context = await context_service.build_context(MemoryUrl("memory://project/test/root"), depth=2) metadata = context["metadata"] - assert metadata["uri"] == "memory://project/test/root" + assert metadata["uri"] == "test/root" assert metadata["depth"] == 2 assert metadata["generated_at"] is not None assert metadata["matched_entities"] > 0 diff --git a/tests/services/test_search_service.py b/tests/services/test_search_service.py index 63948c17..1080c047 100644 --- a/tests/services/test_search_service.py +++ b/tests/services/test_search_service.py @@ -29,6 +29,17 @@ async def test_search_permalink_wildcard(search_service, test_graph): assert "test/root/observations/2" in permalinks +@pytest.mark.skip("search prefix see:'https://sqlite.org/fts5.html#FTS5 Prefix Queries'") +@pytest.mark.asyncio +async def test_search_permalink_wildcard2(search_service, test_graph): + """Pattern matching""" + results = await search_service.search(SearchQuery(permalink_match="test/connected*")) + assert len(results) == 2 + permalinks = {r.permalink for r in results} + assert "test/connected1" in permalinks + assert "test/connected2" in permalinks + + @pytest.mark.asyncio async def test_search_text(search_service, test_graph): """Full-text search"""