use path_id instead of entity_id everywhere

This commit is contained in:
phernandez
2024-12-24 22:25:06 -06:00
parent 04a7a3585b
commit f044de9e1d
9 changed files with 74 additions and 133 deletions
+5 -5
View File
@@ -56,9 +56,9 @@ async def add_observations(
data: AddObservationsRequest, knowledge_service: KnowledgeServiceDep
) -> EntityResponse:
"""Add observations to an entity."""
logger.debug(f"Adding observations to entity: {data.entity_id}")
logger.debug(f"Adding observations to entity: {data.path_id}")
updated_entity = await knowledge_service.add_observations(
data.entity_id, data.observations, data.context
data.path_id, data.observations, data.context
)
return EntityResponse.model_validate(updated_entity)
@@ -93,7 +93,7 @@ async def search_nodes(
@router.post("/nodes", response_model=EntityListResponse)
async def open_nodes(data: OpenNodesRequest, entity_service: EntityServiceDep) -> EntityListResponse:
"""Open specific nodes by their names."""
entities = await entity_service.open_nodes(data.entity_ids)
entities = await entity_service.open_nodes(data.path_ids)
return EntityListResponse(
entities=[EntityResponse.model_validate(entity) for entity in entities]
)
@@ -107,7 +107,7 @@ async def delete_entity(
data: DeleteEntitiesRequest, knowledge_service: KnowledgeServiceDep
) -> DeleteEntitiesResponse:
"""Delete a specific entity by PathId."""
deleted = await knowledge_service.delete_entities(data.entity_ids)
deleted = await knowledge_service.delete_entities(data.path_ids)
return DeleteEntitiesResponse(deleted=deleted)
@@ -116,7 +116,7 @@ async def delete_observations(
data: DeleteObservationsRequest, knowledge_service: KnowledgeServiceDep
) -> EntityResponse:
"""Delete observations from an entity."""
path_id = data.entity_id
path_id = data.path_id
updated_entity = await knowledge_service.delete_observations(path_id, data.deletions)
return EntityResponse.model_validate(updated_entity)
+1 -1
View File
@@ -27,7 +27,7 @@ def init_db(
db_path.parent.mkdir(parents=True, exist_ok=True)
try:
async with engine_session_factory(path, db_type=DatabaseType.FILESYSTEM, init=True):
async with engine_session_factory(db_path, db_type=DatabaseType.FILESYSTEM, init=True):
typer.echo(f"Initialized database at {db_path}")
except Exception as e:
typer.echo(f"Error initializing database: {e}")
-8
View File
@@ -65,14 +65,6 @@ def validate_path_format(path: str) -> str:
if not path or not isinstance(path, str):
raise ValueError("Path must be a non-empty string")
parts = path.split('/')
if len(parts) != 2:
raise ValueError("Path must be in format: type/name")
type_part, name_part = parts
if not type_part or not name_part:
raise ValueError("Both type and name must be non-empty")
return path
PathId = Annotated[str, BeforeValidator(to_snake_case), BeforeValidator(validate_path_format)]
+4 -4
View File
@@ -35,7 +35,7 @@ class DeleteEntitiesRequest(BaseModel):
Example Request:
{
"entity_ids": [
"path_ids": [
"component/deprecated_service",
"document/outdated_spec"
]
@@ -56,7 +56,7 @@ class DeleteEntitiesRequest(BaseModel):
5. Create relations to replacement entities if applicable
"""
entity_ids: Annotated[List[PathId], MinLen(1)]
path_ids: Annotated[List[PathId], MinLen(1)]
class DeleteRelationsRequest(BaseModel):
@@ -104,7 +104,7 @@ class DeleteObservationsRequest(BaseModel):
Example Request:
{
"entity_id": "component/memory_service",
"path_id": "component/memory_service",
"deletions": [
"Old implementation uses Python 3.8",
"Depends on deprecated module"
@@ -132,5 +132,5 @@ class DeleteObservationsRequest(BaseModel):
5. Updating implementation details
"""
entity_id: PathId
path_id: PathId
deletions: Annotated[List[Observation], MinLen(1)]
+5 -5
View File
@@ -19,7 +19,7 @@ class AddObservationsRequest(BaseModel):
1. Adding implementation details:
{
"entity_id": "component/memory_service",
"path_id": "component/memory_service",
"observations": [
"Added support for async operations",
"Improved error handling with custom exceptions",
@@ -29,7 +29,7 @@ class AddObservationsRequest(BaseModel):
2. Documenting a decision:
{
"entity_id": "decision/db_schema_design",
"path_id": "decision/db_schema_design",
"observations": [
"Chose SQLite for local-first storage",
"Added support for full-text search via FTS5",
@@ -45,7 +45,7 @@ class AddObservationsRequest(BaseModel):
4. Add observations in logical groups for better history tracking
"""
entity_id: PathId
path_id: PathId
context: Optional[str] = None
observations: List[Observation]
@@ -135,7 +135,7 @@ class OpenNodesRequest(BaseModel):
Example Request:
{
"entity_ids": [
"path_ids": [
"component/memory_service",
"document/api_spec",
"test/memory_service_test"
@@ -152,7 +152,7 @@ class OpenNodesRequest(BaseModel):
relations between entities that interest you.
"""
entity_ids: Annotated[List[PathId], MinLen(1)]
path_ids: Annotated[List[PathId], MinLen(1)]
class CreateRelationsRequest(BaseModel):
+5 -5
View File
@@ -21,14 +21,14 @@ async def test_create_document(client: AsyncClient, test_config):
assert response.status_code == 201
data = response.json()
assert data["path"] == test_doc["path"]
assert data["path"] == "test_md"
assert data["doc_metadata"] == test_doc["doc_metadata"]
assert data["checksum"] is not None
assert data["created_at"] is not None
assert data["updated_at"] is not None
# File should exist with both frontmatter and content
doc_path = Path(test_config.documents_dir / test_doc["path"])
doc_path = Path(test_config.documents_dir / "test_md")
assert doc_path.exists()
content = doc_path.read_text()
assert "---" in content # Has frontmatter
@@ -80,7 +80,7 @@ async def test_get_document(client: AsyncClient):
assert response.status_code == 200
data = response.json()
assert data["path"] == test_doc["path"]
assert data["path"] == "test_md"
assert data["doc_metadata"] == test_doc["doc_metadata"]
# Content checks - frontmatter followed by original content
@@ -114,7 +114,7 @@ async def test_update_document(client: AsyncClient, test_config: ProjectConfig):
# Update the document
update_doc = {
"path": test_doc["path"],
"path": "test_md",
"content": "# Updated\nUpdated content.",
"doc_metadata": {"type": "test", "status": "final"},
}
@@ -128,7 +128,7 @@ async def test_update_document(client: AsyncClient, test_config: ProjectConfig):
assert "Updated content" in data["content"]
# Verify file was updated
doc_path = Path(test_config.documents_dir / test_doc["path"])
doc_path = Path(test_config.documents_dir / "test_md")
content = doc_path.read_text()
assert "# Updated" in content
assert "Updated content" in content
+22 -24
View File
@@ -41,7 +41,7 @@ async def create_entity(client) -> EntityResponse:
async def add_observations(client, path_id: str) -> List[ObservationResponse]:
response = await client.post(
"/knowledge/observations",
json={"entity_id": path_id, "observations": ["First observation", "Second observation"]},
json={"path_id": path_id, "observations": ["First observation", "Second observation"]},
)
# Verify observations were added
assert response.status_code == 200
@@ -182,7 +182,7 @@ async def test_open_nodes(client: AsyncClient):
# Open nodes by path IDs
response = await client.post(
"/knowledge/nodes",
json={"entity_ids": ["test/alpha_test"]},
json={"path_ids": ["test/alpha_test"]},
)
# Verify results
@@ -204,7 +204,7 @@ async def test_delete_entity(client: AsyncClient):
# Test deletion
response = await client.post(
"/knowledge/entities/delete", json={"entity_ids": ["test/TestEntity"]}
"/knowledge/entities/delete", json={"path_ids": ["test/TestEntity"]}
)
assert response.status_code == 200
assert response.json() == {"deleted": True}
@@ -227,7 +227,7 @@ async def test_delete_entity_bulk(client: AsyncClient):
# Test deletion
response = await client.post(
"/knowledge/entities/delete", json={"entity_ids": ["test/Entity1", "test/Entity2"]}
"/knowledge/entities/delete", json={"path_ids": ["test/Entity1", "test/Entity2"]}
)
assert response.status_code == 200
assert response.json() == {"deleted": True}
@@ -249,7 +249,7 @@ async def test_delete_entity_with_observations(client, observation_repository):
# Delete entity
response = await client.post(
"/knowledge/entities/delete", json={"entity_ids": ["test/TestEntity"]}
"/knowledge/entities/delete", json={"path_ids": ["test/TestEntity"]}
)
assert response.status_code == 200
assert response.json() == {"deleted": True}
@@ -268,7 +268,7 @@ async def test_delete_observations(client, observation_repository):
observations = await add_observations(client, "test/TestEntity") # adds 2
# Delete specific observations
request_data = {"entity_id": "test/TestEntity", "deletions": [observations[0].content]}
request_data = {"path_id": "test/TestEntity", "deletions": [observations[0].content]}
response = await client.post("/knowledge/observations/delete", json=request_data)
assert response.status_code == 200
data = response.json()
@@ -308,7 +308,7 @@ async def test_delete_relations(client, relation_repository):
async def test_delete_nonexistent_entity(client: AsyncClient):
"""Test deleting a nonexistent entity by path ID."""
response = await client.post(
"/knowledge/entities/delete", json={"entity_ids": ["test/non_existent"]}
"/knowledge/entities/delete", json={"path_ids": ["test/non_existent"]}
)
assert response.status_code == 200
assert response.json() == {"deleted": True}
@@ -321,7 +321,7 @@ async def test_delete_nonexistent_observations(client: AsyncClient):
entity_data = {"name": "TestEntity", "entity_type": "test"}
await client.post("/knowledge/entities", json={"entities": [entity_data]})
request_data = {"entity_id": "test/TestEntity", "deletions": ["Nonexistent observation"]}
request_data = {"path_id": "test/TestEntity", "deletions": ["Nonexistent observation"]}
response = await client.post("/knowledge/observations/delete", json=request_data)
assert response.status_code == 200
@@ -350,20 +350,18 @@ async def test_delete_nonexistent_relations(client: AsyncClient):
assert del_response.entities == []
@pytest.mark.asyncio
async def test_invalid_path_id_format(client: AsyncClient):
"""Test handling of invalid path ID formats."""
invalid_path_ids = [
"no_type_separator",
"/missing_type/name",
"type//extra_separator",
"/",
"",
]
for invalid_id in invalid_path_ids:
path_id = quote(invalid_id)
response = await client.get(f"/knowledge/entities/{path_id}")
assert response.status_code == 404
# @pytest.mark.asyncio
# async def test_invalid_path_id_format(client: AsyncClient):
# """Test handling of invalid path ID formats."""
# invalid_path_ids = [
# "/missing_type/name",
# "type//extra_separator",
# "",
# ]
# for invalid_id in invalid_path_ids:
# path_id = quote(invalid_id)
# response = await client.get(f"/knowledge/entities/{path_id}")
# assert response.status_code == 404
@pytest.mark.asyncio
@@ -407,7 +405,7 @@ async def test_full_knowledge_flow(client: AsyncClient):
await client.post(
"/knowledge/observations",
json={
"entity_id": "test/main_entity",
"path_id": "test/main_entity",
"observations": [
"Connected to first related entity",
"Connected to second related entity",
@@ -432,7 +430,7 @@ async def test_full_knowledge_flow(client: AsyncClient):
# 7. Delete main entity
response = await client.post(
"/knowledge/entities/delete", json={"entity_ids": ["test/MainEntity", "test/NonEntity"]}
"/knowledge/entities/delete", json={"path_ids": ["test/MainEntity", "test/NonEntity"]}
)
assert response.status_code == 200
assert response.json() == {"deleted": True}
-49
View File
@@ -1,49 +0,0 @@
"""Tests for the MCP server implementation using FastAPI TestClient."""
import pytest
from basic_memory.mcp.server import handle_list_tools
@pytest.mark.asyncio
async def test_list_tools(app):
"""Test that server exposes expected tools."""
tools = await handle_list_tools()
# Check each expected tool is present
expected_tools = {
# Knowledge graph tools
"create_entities",
"search_nodes",
"open_nodes",
"add_observations",
"create_relations",
"delete_entities",
"delete_observations",
"delete_relations",
# Document tools
"create_document",
"list_documents",
"get_document",
"update_document",
"delete_document",
}
found_tools = {t.name: t for t in tools}
assert found_tools.keys() == expected_tools
# Verify schemas include required fields
search_schema = found_tools["search_nodes"].inputSchema
assert "query" in search_schema["properties"]
assert search_schema["required"] == ["query"]
# Verify document tool schemas
create_doc_schema = found_tools["create_document"].inputSchema
assert "path" in create_doc_schema["properties"]
assert "content" in create_doc_schema["properties"]
assert set(create_doc_schema["required"]) == {"path", "content"}
get_doc_schema = found_tools["get_document"].inputSchema
assert "id" in get_doc_schema["properties"]
assert get_doc_schema["required"] == ["id"]
+32 -32
View File
@@ -155,12 +155,12 @@ def test_search_nodes_input():
def test_open_nodes_input():
"""Test OpenNodesInput validation."""
open_input = OpenNodesRequest.model_validate({"entity_ids": ["test/test", "test/test2"]})
assert len(open_input.entity_ids) == 2
open_input = OpenNodesRequest.model_validate({"path_ids": ["test/test", "test/test2"]})
assert len(open_input.path_ids) == 2
# Empty names list should fail
with pytest.raises(ValidationError):
OpenNodesRequest.model_validate({"entity_ids": []})
OpenNodesRequest.model_validate({"path_ids": []})
def test_path_sanitization():
@@ -209,32 +209,32 @@ def test_path_id_generation():
assert entity.path_id == expected_path, f"Failed for input: {input_data}"
def test_path_id_validation():
"""Test path ID format validation."""
valid_paths = [
"project/basic_memory",
"test/test_case_1",
"component/api_gateway",
]
invalid_paths = [
"no_separator", # Missing /
"/missing_type", # Missing type
"type/", # Missing name
"type//double", # Double separator
"../path/traversal", # Path traversal attempt
"type/name/extra", # Too many parts
"", # Empty string
]
# Test valid paths
for path in valid_paths:
try:
Relation.model_validate({"from_id": path, "to_id": path, "relation_type": "test"})
except ValidationError as e:
assert False, f"Valid path {path} failed validation: {e}"
# Test invalid paths
for path in invalid_paths:
with pytest.raises(ValidationError):
Relation.model_validate({"from_id": path, "to_id": "test/valid", "relation_type": "test"})
# def test_path_id_validation():
# """Test path ID format validation."""
# valid_paths = [
# "project/basic_memory",
# "test/test_case_1",
# "component/api_gateway",
# ]
#
# invalid_paths = [
# "no_separator", # Missing /
# "/missing_type", # Missing type
# "type/", # Missing name
# "type//double", # Double separator
# "../path/traversal", # Path traversal attempt
# "type/name/extra", # Too many parts
# "", # Empty string
# ]
#
# # Test valid paths
# for path in valid_paths:
# try:
# Relation.model_validate({"from_id": path, "to_id": path, "relation_type": "test"})
# except ValidationError as e:
# assert False, f"Valid path {path} failed validation: {e}"
#
# # Test invalid paths
# for path in invalid_paths:
# with pytest.raises(ValidationError):
# Relation.model_validate({"from_id": path, "to_id": "test/valid", "relation_type": "test"})