fix test coverage and type checks

Signed-off-by: phernandez <paul@basicmachines.co>
This commit is contained in:
phernandez
2025-03-24 22:45:56 -05:00
parent e716946b44
commit b667bca5a2
12 changed files with 134 additions and 130 deletions
+4 -3
View File
@@ -98,15 +98,16 @@ def set_default_project(
try:
# Set the default project
config_manager.set_default_project(name)
# Also activate it for the current session by setting the environment variable
os.environ["BASIC_MEMORY_PROJECT"] = name
# Reload configuration to apply the change
from importlib import reload
from basic_memory import config as config_module
reload(config_module)
console.print(f"[green]Project '{name}' set as default and activated[/green]")
except ValueError as e: # pragma: no cover
console.print(f"[red]Error: {e}[/red]")
+13 -10
View File
@@ -4,21 +4,22 @@ Uses markdown-it with plugins to parse structured data from markdown content.
"""
from dataclasses import dataclass, field
from pathlib import Path
from datetime import datetime
from pathlib import Path
from typing import Any, Optional
import dateparser
from markdown_it import MarkdownIt
import dateparser
import frontmatter
from markdown_it import MarkdownIt
from basic_memory.markdown.plugins import observation_plugin, relation_plugin
from basic_memory.markdown.schemas import (
EntityMarkdown,
EntityFrontmatter,
EntityMarkdown,
Observation,
Relation,
)
from basic_memory.utils import parse_tags
md = MarkdownIt().use(observation_plugin).use(relation_plugin)
@@ -56,11 +57,11 @@ def parse(content: str) -> EntityContent:
)
def parse_tags(tags: Any) -> list[str]:
"""Parse tags into list of strings."""
if isinstance(tags, (list, tuple)):
return [str(t).strip() for t in tags if str(t).strip()]
return [t.strip() for t in tags.split(",") if t.strip()]
# def parse_tags(tags: Any) -> list[str]:
# """Parse tags into list of strings."""
# if isinstance(tags, (list, tuple)):
# return [str(t).strip() for t in tags if str(t).strip()]
# return [t.strip() for t in tags.split(",") if t.strip()]
class EntityParser:
@@ -101,7 +102,9 @@ class EntityParser:
metadata = post.metadata
metadata["title"] = post.metadata.get("title", absolute_path.name)
metadata["type"] = post.metadata.get("type", "note")
metadata["tags"] = parse_tags(post.metadata.get("tags", []))
tags = parse_tags(post.metadata.get("tags", [])) # pyright: ignore
if tags:
metadata["tags"] = tags
# frontmatter
entity_frontmatter = EntityFrontmatter(
+1 -1
View File
@@ -42,7 +42,7 @@ class EntityFrontmatter(BaseModel):
@property
def tags(self) -> List[str]:
return self.metadata.get("tags") if self.metadata else [] # pyright: ignore
return self.metadata.get("tags") if self.metadata else None # pyright: ignore
@property
def title(self) -> str:
-27
View File
@@ -5,7 +5,6 @@ to the Basic Memory API, with improved error handling and logging.
"""
import typing
from typing import Union, List
from httpx import Response, URL, AsyncClient, HTTPStatusError
from httpx._client import UseClientDefault, USE_CLIENT_DEFAULT
@@ -24,32 +23,6 @@ from loguru import logger
from mcp.server.fastmcp.exceptions import ToolError
def parse_tags(tags: Union[List[str], str, None]) -> List[str]:
"""Parse tags from various input formats into a consistent list.
Args:
tags: Can be a list of strings, a comma-separated string, or None
Returns:
A list of tag strings, or an empty list if no tags
"""
if tags is None:
return []
if isinstance(tags, list):
return tags
if isinstance(tags, str):
return [tag.strip() for tag in tags.split(",") if tag.strip()]
# For any other type, try to convert to string and parse
try:
return parse_tags(str(tags))
except (ValueError, TypeError):
logger.warning(f"Couldn't parse tags from input of type {type(tags)}: {tags}")
return []
def get_error_message(status_code: int, url: URL | str, method: str) -> str:
"""Get a friendly error message based on the HTTP status code.
+4 -3
View File
@@ -6,9 +6,10 @@ from loguru import logger
from basic_memory.mcp.async_client import client
from basic_memory.mcp.server import mcp
from basic_memory.mcp.tools.utils import call_put, parse_tags
from basic_memory.mcp.tools.utils import call_put
from basic_memory.schemas import EntityResponse
from basic_memory.schemas.base import Entity
from basic_memory.utils import parse_tags
# Define TagType as a Union that can accept either a string or a list of strings or None
TagType = Union[List[str], str, None]
@@ -21,7 +22,7 @@ async def write_note(
title: str,
content: str,
folder: str,
tags = None, # Remove type hint completely to avoid schema issues
tags=None, # Remove type hint completely to avoid schema issues
) -> str:
"""Write a markdown note to the knowledge base.
@@ -64,7 +65,7 @@ async def write_note(
# Process tags using the helper function
tag_list = parse_tags(tags)
# Create the entity request
metadata = {"tags": [f"#{tag}" for tag in tag_list]} if tag_list else None
entity = Entity(
+4 -7
View File
@@ -146,15 +146,12 @@ class EntityService(BaseService[EntityModel]):
# Create post with new content from schema
post = await schema_to_markdown(schema)
# Merge new metadata with existing metadata
existing_markdown.frontmatter.metadata.update(post.metadata)
# Create a new post with merged metadata
merged_post = frontmatter.Post(
post.content,
**existing_markdown.frontmatter.metadata
)
merged_post = frontmatter.Post(post.content, **existing_markdown.frontmatter.metadata)
# write file
final_content = frontmatter.dumps(merged_post, sort_keys=False)
@@ -322,4 +319,4 @@ class EntityService(BaseService[EntityModel]):
)
continue
return await self.repository.get_by_file_path(path)
return await self.repository.get_by_file_path(path)
+1 -1
View File
@@ -351,4 +351,4 @@ class WatchService:
duration_ms=duration_ms,
)
await self.write_status()
await self.write_status()
+27 -1
View File
@@ -6,7 +6,7 @@ import logging
import re
import sys
from pathlib import Path
from typing import Optional, Protocol, Union, runtime_checkable
from typing import Optional, Protocol, Union, runtime_checkable, List
from loguru import logger
from unidecode import unidecode
@@ -128,3 +128,29 @@ def setup_logging(
# Set log levels for noisy loggers
for logger_name, level in noisy_loggers.items():
logging.getLogger(logger_name).setLevel(level)
def parse_tags(tags: Union[List[str], str, None]) -> List[str]:
"""Parse tags from various input formats into a consistent list.
Args:
tags: Can be a list of strings, a comma-separated string, or None
Returns:
A list of tag strings, or an empty list if no tags
"""
if tags is None:
return []
if isinstance(tags, list):
return tags
if isinstance(tags, str):
return [tag.strip() for tag in tags.split(",") if tag.strip()]
# For any other type, try to convert to string and parse
try: # pragma: no cover
return parse_tags(str(tags))
except (ValueError, TypeError): # pragma: no cover
logger.warning(f"Couldn't parse tags from input of type {type(tags)}: {tags}")
return []
+8 -8
View File
@@ -17,11 +17,11 @@ def temp_home(monkeypatch):
"""Create a temporary directory for testing."""
# Save the original environment variable if it exists
original_env = os.environ.get("BASIC_MEMORY_PROJECT")
# Clear environment variable for clean test
if "BASIC_MEMORY_PROJECT" in os.environ:
del os.environ["BASIC_MEMORY_PROJECT"]
with TemporaryDirectory() as tempdir:
temp_home = Path(tempdir)
monkeypatch.setattr(Path, "home", lambda: temp_home)
@@ -31,7 +31,7 @@ def temp_home(monkeypatch):
config_dir.mkdir(parents=True, exist_ok=True)
yield temp_home
# Cleanup: restore original environment variable if it existed
if original_env is not None:
os.environ["BASIC_MEMORY_PROJECT"] = original_env
@@ -141,7 +141,7 @@ def test_project_default(cli_runner, temp_home):
# Verify default was set
config_manager = ConfigManager()
assert config_manager.default_project == "test"
# Extra verification: check if the environment variable was set
assert os.environ.get("BASIC_MEMORY_PROJECT") == "test"
@@ -157,7 +157,7 @@ def test_project_current(cli_runner, temp_home):
"default_project": "main",
}
config_file.write_text(json.dumps(config_data))
# Create the main project directory
main_dir = temp_home / "basic-memory"
main_dir.mkdir(parents=True, exist_ok=True)
@@ -192,16 +192,16 @@ def test_project_default_activates_project(cli_runner, temp_home, monkeypatch):
# Create a test environment
env = {}
monkeypatch.setattr(os, "environ", env)
# Create two test projects
config_manager = ConfigManager()
config_manager.add_project("project1", str(temp_home / "project1"))
# Set project1 as default using the CLI command
result = cli_runner.invoke(app, ["project", "default", "project1"])
assert result.exit_code == 0
assert "Project 'project1' set as default and activated" in result.stdout
# Verify the environment variable was set
# This is the core of our fix - the set_default_project command now also sets
# the BASIC_MEMORY_PROJECT environment variable to activate the project
+18 -19
View File
@@ -189,7 +189,7 @@ async def test_delete_note_doesnt_exist(app):
@pytest.mark.asyncio
async def test_write_note_with_tag_array_from_bug_report(app):
"""Test creating a note with a tag array as reported in issue #38.
This reproduces the exact payload from the bug report where Cursor
was passing an array of tags and getting a type mismatch error.
"""
@@ -198,12 +198,12 @@ async def test_write_note_with_tag_array_from_bug_report(app):
"title": "Title",
"folder": "folder",
"content": "CONTENT",
"tags": ["hipporag", "search", "fallback", "symfony", "error-handling"]
"tags": ["hipporag", "search", "fallback", "symfony", "error-handling"],
}
# Try to call the function with this data directly
result = await write_note(**bug_payload)
assert result
assert "permalink: folder/title" in result
assert "Tags" in result
@@ -257,10 +257,10 @@ async def test_write_note_verbose(app):
@pytest.mark.asyncio
async def test_write_note_preserves_custom_metadata(app, test_config):
"""Test that updating a note preserves custom metadata fields.
Reproduces issue #36 where custom frontmatter fields like Status
were being lost when updating notes with the write_note tool.
Should:
- Create a note with custom frontmatter
- Update the note with new content
@@ -273,15 +273,14 @@ async def test_write_note_preserves_custom_metadata(app, test_config):
content="# Initial content",
tags=["test"],
)
# Read the note to get its permalink
content = await read_note("test/custom-metadata-note")
# Now directly update the file with custom frontmatter
# We need to use a direct file update to add custom frontmatter
from pathlib import Path
import frontmatter
file_path = test_config.home / "test" / "Custom Metadata Note.md"
post = frontmatter.load(file_path)
@@ -289,11 +288,11 @@ async def test_write_note_preserves_custom_metadata(app, test_config):
post["Status"] = "In Progress"
post["Priority"] = "High"
post["Version"] = "1.0"
# Write the file back
with open(file_path, "w") as f:
f.write(frontmatter.dumps(post))
# Now update the note using write_note
result = await write_note(
title="Custom Metadata Note",
@@ -301,23 +300,23 @@ async def test_write_note_preserves_custom_metadata(app, test_config):
content="# Updated content",
tags=["test", "updated"],
)
# Verify the update was successful
assert "Updated test/Custom Metadata Note.md" in result
# Read the note back and check if custom frontmatter is preserved
content = await read_note("test/custom-metadata-note")
# Custom frontmatter should be preserved
assert "Status: In Progress" in content
assert "Priority: High" in content
# Version might be quoted as '1.0' due to YAML serialization
assert "Version:" in content # Just check that the field exists
assert "1.0" in content # And that the value exists somewhere
assert "1.0" in content # And that the value exists somewhere
# And new content should be there
assert "# Updated content" in content
# And tags should be updated
assert "'#test'" in content
assert "'#updated'" in content
assert "'#updated'" in content
+1 -1
View File
@@ -521,7 +521,7 @@ async def test_update_with_content(entity_service: EntityService, file_service:
"""
).strip()
# Create test entity
# update entity
entity, created = await entity_service.create_or_update_entity(
EntitySchema(
title="Git Workflow Guide",
+53 -49
View File
@@ -7,7 +7,6 @@ import pytest
from watchfiles import Change
async def create_test_file(path: Path, content: str = "test content") -> None:
"""Create a test file with given content."""
path.parent.mkdir(parents=True, exist_ok=True)
@@ -20,9 +19,9 @@ async def test_temp_file_filter(watch_service):
# Test filter_changes method directly
tmp_path = str(watch_service.config.home / "test.tmp")
assert not watch_service.filter_changes(Change.added, tmp_path)
# Test with valid file
valid_path = str(watch_service.config.home / "test.md")
valid_path = str(watch_service.config.home / "test.md")
assert watch_service.filter_changes(Change.added, valid_path)
@@ -30,44 +29,44 @@ async def test_temp_file_filter(watch_service):
async def test_handle_tmp_files(watch_service, test_config, monkeypatch):
"""Test handling of .tmp files during sync process."""
project_dir = test_config.home
# Create a .tmp file - this simulates a file being written with write_file_atomic
tmp_file = project_dir / "test.tmp"
await create_test_file(tmp_file, "This is a temporary file")
# Create the target final file
final_file = project_dir / "test.md"
await create_test_file(final_file, "This is the final file")
# Setup changes that include both the .tmp and final file
changes = {
(Change.added, str(tmp_file)),
(Change.added, str(final_file)),
}
# Track sync_file calls
sync_calls = []
# Mock sync_file to track calls
original_sync_file = watch_service.sync_service.sync_file
async def mock_sync_file(path, new=True):
sync_calls.append(path)
return await original_sync_file(path, new)
monkeypatch.setattr(watch_service.sync_service, "sync_file", mock_sync_file)
# Handle changes
await watch_service.handle_changes(project_dir, changes)
# Verify .tmp file was not processed
assert "test.tmp" not in sync_calls
assert "test.md" in sync_calls
# Verify only the final file got an entity
tmp_entity = await watch_service.sync_service.entity_repository.get_by_file_path("test.tmp")
final_entity = await watch_service.sync_service.entity_repository.get_by_file_path("test.md")
assert tmp_entity is None, "Temp file should not have an entity"
assert final_entity is not None, "Final file should have an entity"
@@ -76,44 +75,43 @@ async def test_handle_tmp_files(watch_service, test_config, monkeypatch):
async def test_atomic_write_tmp_file_handling(watch_service, test_config, monkeypatch):
"""Test handling of file changes during atomic write operations."""
project_dir = test_config.home
# This test simulates the full atomic write process:
# 1. First a .tmp file is created
# 2. Then the .tmp file is renamed to the final file
# 2. Then the .tmp file is renamed to the final file
# 3. Both events are processed by the watch service
# Setup file paths
tmp_path = project_dir / "document.tmp"
final_path = project_dir / "document.md"
# Create mockup of the atomic write process
await create_test_file(tmp_path, "Content for document")
# First batch of changes - .tmp file created
changes1 = {(Change.added, str(tmp_path))}
# Process first batch
await watch_service.handle_changes(project_dir, changes1)
# Now "replace" the temp file with the final file
tmp_path.rename(final_path)
# Second batch of changes - .tmp file deleted, final file added
changes2 = {
(Change.deleted, str(tmp_path)),
(Change.added, str(final_path))
}
changes2 = {(Change.deleted, str(tmp_path)), (Change.added, str(final_path))}
# Process second batch
await watch_service.handle_changes(project_dir, changes2)
# Verify only the final file is in the database
tmp_entity = await watch_service.sync_service.entity_repository.get_by_file_path("document.tmp")
final_entity = await watch_service.sync_service.entity_repository.get_by_file_path("document.md")
final_entity = await watch_service.sync_service.entity_repository.get_by_file_path(
"document.md"
)
assert tmp_entity is None, "Temp file should not have an entity"
assert final_entity is not None, "Final file should have an entity"
# Check events
new_events = [e for e in watch_service.state.recent_events if e.action == "new"]
assert len(new_events) == 1
@@ -124,57 +122,63 @@ async def test_atomic_write_tmp_file_handling(watch_service, test_config, monkey
async def test_rapid_atomic_writes(watch_service, test_config):
"""Test handling of rapid atomic writes to the same destination."""
project_dir = test_config.home
# This test simulates multiple rapid atomic writes to the same file:
# 1. Several .tmp files are created one after another
# 1. Several .tmp files are created one after another
# 2. Each is then renamed to the same final file
# 3. Events are batched and processed together
# Setup file paths
tmp1_path = project_dir / "document.1.tmp"
tmp2_path = project_dir / "document.2.tmp"
tmp2_path = project_dir / "document.2.tmp"
final_path = project_dir / "document.md"
# Create multiple temp files that will be used in sequence
await create_test_file(tmp1_path, "First version")
await create_test_file(tmp2_path, "Second version")
# Simulate the first atomic write
tmp1_path.rename(final_path)
# Brief pause to ensure file system registers the change
await asyncio.sleep(0.1)
# Read content to verify
content1 = final_path.read_text()
assert content1 == "First version"
# Simulate the second atomic write
tmp2_path.rename(final_path)
# Verify content was updated
content2 = final_path.read_text()
assert content2 == "Second version"
# Create a batch of changes that might arrive in mixed order
changes = {
(Change.added, str(tmp1_path)),
(Change.deleted, str(tmp1_path)),
(Change.added, str(tmp2_path)),
(Change.added, str(tmp2_path)),
(Change.deleted, str(tmp2_path)),
(Change.added, str(final_path)),
(Change.modified, str(final_path)),
}
# Process all changes
await watch_service.handle_changes(project_dir, changes)
# Verify only the final file is in the database
final_entity = await watch_service.sync_service.entity_repository.get_by_file_path("document.md")
final_entity = await watch_service.sync_service.entity_repository.get_by_file_path(
"document.md"
)
assert final_entity is not None
# Also verify no tmp entities were created
tmp1_entity = await watch_service.sync_service.entity_repository.get_by_file_path("document.1.tmp")
tmp2_entity = await watch_service.sync_service.entity_repository.get_by_file_path("document.2.tmp")
tmp1_entity = await watch_service.sync_service.entity_repository.get_by_file_path(
"document.1.tmp"
)
tmp2_entity = await watch_service.sync_service.entity_repository.get_by_file_path(
"document.2.tmp"
)
assert tmp1_entity is None
assert tmp2_entity is None