Skip to content

Commit 4a43d7d

Browse files
committed
remove logfire instrumentation
Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 70bb10b commit 4a43d7d

43 files changed

Lines changed: 573 additions & 1194 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.claude/settings.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"enabledPlugins": {
3+
"basic-memory@basicmachines": true
4+
}
5+
}

.github/workflows/test.yml

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -78,21 +78,7 @@ jobs:
7878
python-version: [ "3.12", "3.13" ]
7979
runs-on: ubuntu-latest
8080

81-
# Postgres service (only available on Linux runners)
82-
services:
83-
postgres:
84-
image: postgres:17
85-
env:
86-
POSTGRES_DB: basic_memory_test
87-
POSTGRES_USER: basic_memory_user
88-
POSTGRES_PASSWORD: dev_password
89-
options: >-
90-
--health-cmd pg_isready
91-
--health-interval 10s
92-
--health-timeout 5s
93-
--health-retries 5
94-
ports:
95-
- 5433:5432
81+
# Note: No services section needed - testcontainers handles Postgres in Docker
9682

9783
steps:
9884
- uses: actions/checkout@v4
@@ -121,7 +107,7 @@ jobs:
121107
run: |
122108
uv pip install -e .[dev]
123109
124-
- name: Run tests (Postgres)
110+
- name: Run tests (Postgres via testcontainers)
125111
run: |
126112
uv pip install pytest pytest-cov
127113
just test-postgres

CLAUDE.md

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,14 @@ See the [README.md](README.md) file for a project overview.
1515
### Build and Test Commands
1616

1717
- Install: `just install` or `pip install -e ".[dev]"`
18-
- Run all tests (with coverage): `just test` - Runs both unit and integration tests with unified coverage
19-
- Run unit tests only: `just test-unit` - Fast, no coverage
20-
- Run integration tests only: `just test-int` - Fast, no coverage
21-
- Generate HTML coverage: `just coverage` - Opens in browser
18+
- Run all tests (SQLite + Postgres): `just test`
19+
- Run all tests against SQLite: `just test-sqlite`
20+
- Run all tests against Postgres: `just test-postgres` (uses testcontainers)
21+
- Run unit tests (SQLite): `just test-unit-sqlite`
22+
- Run unit tests (Postgres): `just test-unit-postgres`
23+
- Run integration tests (SQLite): `just test-int-sqlite`
24+
- Run integration tests (Postgres): `just test-int-postgres`
25+
- Generate HTML coverage: `just coverage`
2226
- Single test: `pytest tests/path/to/test_file.py::test_function_name`
2327
- Run benchmarks: `pytest test-int/test_sync_performance_benchmark.py -v -m "benchmark and not slow"`
2428
- Lint: `just lint` or `ruff check . --fix`
@@ -30,6 +34,8 @@ See the [README.md](README.md) file for a project overview.
3034

3135
**Note:** Project requires Python 3.12+ (uses type parameter syntax and `type` aliases introduced in 3.12)
3236

37+
**Postgres Testing:** Uses [testcontainers](https://testcontainers-python.readthedocs.io/) which automatically spins up a Postgres instance in Docker. No manual database setup required - just have Docker running.
38+
3339
### Test Structure
3440

3541
- `tests/` - Unit tests for individual components (mocked, fast)
@@ -76,8 +82,10 @@ See the [README.md](README.md) file for a project overview.
7682
- SQLite is used for indexing and full text search, files are source of truth
7783
- Testing uses pytest with asyncio support (strict mode)
7884
- Unit tests (`tests/`) use mocks when necessary; integration tests (`test-int/`) use real implementations
79-
- Test database uses in-memory SQLite
80-
- Each test runs in a standalone environment with in-memory SQLite and tmp_file directory
85+
- By default, tests run against SQLite (fast, no Docker needed)
86+
- Set `BASIC_MEMORY_TEST_POSTGRES=1` to run against Postgres (uses testcontainers - Docker required)
87+
- Each test runs in a standalone environment with isolated database and tmp_path directory
88+
- CI runs SQLite and Postgres tests in parallel for faster feedback
8189
- Performance benchmarks are in `test-int/test_sync_performance_benchmark.py`
8290
- Use pytest markers: `@pytest.mark.benchmark` for benchmarks, `@pytest.mark.slow` for slow tests
8391

@@ -229,6 +237,11 @@ of using AI just for code generation, we've developed a true collaborative workf
229237
This approach has allowed us to tackle more complex challenges and build a more robust system than either humans or AI
230238
could achieve independently.
231239

240+
**Problem-Solving Guidance:**
241+
- If a solution isn't working after reasonable effort, suggest alternative approaches
242+
- Don't persist with a problematic library or pattern when better alternatives exist
243+
- Example: When py-pglite caused cascading test failures, switching to testcontainers-postgres was the right call
244+
232245
## GitHub Integration
233246

234247
Basic Memory has taken AI-Human collaboration to the next level by integrating Claude directly into the development workflow through GitHub:

README.md

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -437,38 +437,39 @@ See the [Documentation](https://memory.basicmachines.co/) for more info, includi
437437

438438
### Running Tests
439439

440-
Basic Memory supports dual database backends (SQLite and Postgres). Tests are parametrized to run against both backends automatically.
440+
Basic Memory supports dual database backends (SQLite and Postgres). By default, tests run against SQLite. Set `BASIC_MEMORY_TEST_POSTGRES=1` to run against Postgres (uses testcontainers - Docker required).
441441

442442
**Quick Start:**
443443
```bash
444-
# Run SQLite tests (default, no Docker needed)
444+
# Run all tests against SQLite (default, fast)
445445
just test-sqlite
446446

447-
# Run Postgres tests (requires Docker)
447+
# Run all tests against Postgres (uses testcontainers)
448448
just test-postgres
449+
450+
# Run both SQLite and Postgres tests
451+
just test
449452
```
450453

451454
**Available Test Commands:**
452455

453-
- `just test-sqlite` - Run tests against SQLite only (fastest, no Docker needed)
454-
- `just test-postgres` - Run tests against Postgres only (requires Docker)
456+
- `just test` - Run all tests against both SQLite and Postgres
457+
- `just test-sqlite` - Run all tests against SQLite (fast, no Docker needed)
458+
- `just test-postgres` - Run all tests against Postgres (uses testcontainers)
459+
- `just test-unit-sqlite` - Run unit tests against SQLite
460+
- `just test-unit-postgres` - Run unit tests against Postgres
461+
- `just test-int-sqlite` - Run integration tests against SQLite
462+
- `just test-int-postgres` - Run integration tests against Postgres
455463
- `just test-windows` - Run Windows-specific tests (auto-skips on other platforms)
456464
- `just test-benchmark` - Run performance benchmark tests
457-
- `just test-all` - Run all tests including Windows, Postgres, and benchmarks
458-
459-
**Postgres Testing Requirements:**
460465

461-
To run Postgres tests, you need to start the test database:
462-
```bash
463-
docker-compose -f docker-compose-postgres.yml up -d
464-
```
466+
**Postgres Testing:**
465467

466-
Tests will connect to `localhost:5433/basic_memory_test`.
468+
Postgres tests use [testcontainers](https://testcontainers-python.readthedocs.io/) which automatically spins up a Postgres instance in Docker. No manual database setup required - just have Docker running.
467469

468470
**Test Markers:**
469471

470472
Tests use pytest markers for selective execution:
471-
- `postgres` - Tests that run against Postgres backend
472473
- `windows` - Windows-specific database optimizations
473474
- `benchmark` - Performance tests (excluded from default runs)
474475

docker-compose-postgres.yml

Lines changed: 0 additions & 42 deletions
This file was deleted.

justfile

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -7,44 +7,51 @@ install:
77
@echo ""
88
@echo "💡 Remember to activate the virtual environment by running: source .venv/bin/activate"
99

10-
# Run all tests with unified coverage report
11-
test: test-unit test-int
12-
13-
# Run unit tests only (fast, no coverage)
14-
test-unit:
15-
uv run pytest -p pytest_mock -v --no-cov tests
16-
17-
# Run integration tests only (fast, no coverage)
18-
test-int:
19-
uv run pytest -p pytest_mock -v --no-cov test-int
20-
2110
# ==============================================================================
2211
# DATABASE BACKEND TESTING
2312
# ==============================================================================
2413
# Basic Memory supports dual database backends (SQLite and Postgres).
25-
# Tests are parametrized to run against both backends automatically.
14+
# By default, tests run against SQLite (fast, no dependencies).
15+
# Set BASIC_MEMORY_TEST_POSTGRES=1 to run against Postgres (uses testcontainers).
2616
#
2717
# Quick Start:
28-
# just test-sqlite # Run SQLite tests (default, no Docker needed)
29-
# just test-postgres # Run Postgres tests (requires Docker)
18+
# just test # Run all tests against SQLite (default)
19+
# just test-sqlite # Run all tests against SQLite
20+
# just test-postgres # Run all tests against Postgres (testcontainers)
21+
# just test-unit-sqlite # Run unit tests against SQLite
22+
# just test-unit-postgres # Run unit tests against Postgres
23+
# just test-int-sqlite # Run integration tests against SQLite
24+
# just test-int-postgres # Run integration tests against Postgres
3025
#
31-
# For Postgres tests, first start the database:
32-
# docker-compose -f docker-compose-postgres.yml up -d
26+
# CI runs both in parallel for faster feedback.
3327
# ==============================================================================
3428

35-
# Run tests against SQLite only (default backend, skip Postgres/Benchmark tests)
36-
# This is the fastest option and doesn't require any Docker setup.
37-
# Use this for local development and quick feedback.
38-
# Includes Windows-specific tests which will auto-skip on non-Windows platforms.
39-
test-sqlite:
40-
uv run pytest -p pytest_mock -v --no-cov -m "not postgres and not benchmark" tests test-int
41-
42-
# Run tests against Postgres only (requires docker-compose-postgres.yml up)
43-
# First start Postgres: docker-compose -f docker-compose-postgres.yml up -d
44-
# Tests will connect to localhost:5433/basic_memory_test
45-
# To reset the database: just postgres-reset
46-
test-postgres:
47-
uv run pytest -p pytest_mock -v --no-cov -m "postgres and not benchmark" tests test-int
29+
# Run all tests against SQLite and Postgres
30+
test: test-sqlite test-postgres
31+
32+
# Run all tests against SQLite
33+
test-sqlite: test-unit-sqlite test-int-sqlite
34+
35+
# Run all tests against Postgres (uses testcontainers)
36+
test-postgres: test-unit-postgres test-int-postgres
37+
38+
# Run unit tests against SQLite
39+
test-unit-sqlite:
40+
uv run pytest -p pytest_mock -v --no-cov tests
41+
42+
# Run unit tests against Postgres
43+
test-unit-postgres:
44+
BASIC_MEMORY_TEST_POSTGRES=1 uv run pytest -p pytest_mock -v --no-cov tests
45+
46+
# Run integration tests against SQLite
47+
test-int-sqlite:
48+
uv run pytest -p pytest_mock -v --no-cov test-int
49+
50+
# Run integration tests against Postgres
51+
# Note: Uses timeout due to FastMCP Client + asyncpg cleanup hang (tests pass, process hangs on exit)
52+
# See: https://github.com/jlowin/fastmcp/issues/1311
53+
test-int-postgres:
54+
timeout --signal=KILL 300 bash -c 'BASIC_MEMORY_TEST_POSTGRES=1 uv run pytest -p pytest_mock -v --no-cov test-int' || test $? -eq 137
4855

4956
# Reset Postgres test database (drops and recreates schema)
5057
# Useful when Alembic migration state gets out of sync during development

pyproject.toml

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,11 @@ dependencies = [
3333
"pyjwt>=2.10.1",
3434
"python-dotenv>=1.1.0",
3535
"pytest-aio>=1.9.0",
36-
"aiofiles>=24.1.0", # Async file I/O
37-
"logfire[fastapi]>=0.73.0", # Optional observability (disabled by default via config)
36+
"aiofiles>=24.1.0", # Optional observability (disabled by default via config)
3837
"asyncpg>=0.30.0",
3938
"nest-asyncio>=1.6.0", # For Alembic migrations with Postgres
39+
"pytest-asyncio>=1.2.0",
40+
"psycopg==3.3.1",
4041
]
4142

4243

@@ -81,7 +82,8 @@ dev = [
8182
"pytest-xdist>=3.0.0",
8283
"ruff>=0.1.6",
8384
"freezegun>=1.5.5",
84-
85+
"testcontainers[postgres]>=4.0.0",
86+
"psycopg>=3.2.0",
8587
]
8688

8789
[tool.hatch.version]

src/basic_memory/markdown/entity_parser.py

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,11 @@
2222
Relation,
2323
)
2424
from basic_memory.utils import parse_tags
25-
import logfire
25+
2626

2727
md = MarkdownIt().use(observation_plugin).use(relation_plugin)
2828

2929

30-
@logfire.instrument()
3130
def normalize_frontmatter_value(value: Any) -> Any:
3231
"""Normalize frontmatter values to safe types for processing.
3332
@@ -89,7 +88,6 @@ def normalize_frontmatter_value(value: Any) -> Any:
8988
return value
9089

9190

92-
@logfire.instrument()
9391
def normalize_frontmatter_metadata(metadata: dict) -> dict:
9492
"""Normalize all values in frontmatter metadata dict.
9593
@@ -112,7 +110,6 @@ class EntityContent:
112110
relations: list[Relation] = field(default_factory=list)
113111

114112

115-
@logfire.instrument()
116113
def parse(content: str) -> EntityContent:
117114
"""Parse markdown content into EntityMarkdown."""
118115

@@ -171,7 +168,6 @@ def parse_date(self, value: Any) -> Optional[datetime]:
171168
return parsed
172169
return None
173170

174-
@logfire.instrument()
175171
async def parse_file(self, path: Path | str) -> EntityMarkdown:
176172
"""Parse markdown file into EntityMarkdown."""
177173

@@ -193,7 +189,6 @@ def get_file_path(self, path):
193189
"""Get absolute path for a file using the base path for the project."""
194190
return self.base_path / path
195191

196-
@logfire.instrument()
197192
async def parse_file_content(self, absolute_path, file_content):
198193
"""Parse markdown content from file stats.
199194
@@ -211,7 +206,6 @@ async def parse_file_content(self, absolute_path, file_content):
211206
ctime=file_stats.st_ctime,
212207
)
213208

214-
@logfire.instrument()
215209
async def parse_markdown_content(
216210
self,
217211
file_path: Path,

src/basic_memory/markdown/markdown_processor.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
from frontmatter import Post
66
from loguru import logger
7-
import logfire
7+
88

99
from basic_memory import file_utils
1010
from basic_memory.file_utils import dump_frontmatter
@@ -40,7 +40,6 @@ def __init__(self, entity_parser: EntityParser):
4040
"""Initialize processor with base path and parser."""
4141
self.entity_parser = entity_parser
4242

43-
@logfire.instrument()
4443
async def read_file(self, path: Path) -> EntityMarkdown:
4544
"""Read and parse file into EntityMarkdown schema.
4645
@@ -49,7 +48,6 @@ async def read_file(self, path: Path) -> EntityMarkdown:
4948
"""
5049
return await self.entity_parser.parse_file(path)
5150

52-
@logfire.instrument()
5351
async def write_file(
5452
self,
5553
path: Path,
@@ -127,7 +125,6 @@ async def write_file(
127125
await file_utils.write_file_atomic(path, final_content)
128126
return await file_utils.compute_checksum(final_content)
129127

130-
@logfire.instrument()
131128
def format_observations(self, observations: list[Observation]) -> str:
132129
"""Format observations section in standard way.
133130
@@ -136,7 +133,6 @@ def format_observations(self, observations: list[Observation]) -> str:
136133
lines = [f"{obs}" for obs in observations]
137134
return "\n".join(lines) + "\n"
138135

139-
@logfire.instrument()
140136
def format_relations(self, relations: list[Relation]) -> str:
141137
"""Format relations section in standard way.
142138

0 commit comments

Comments
 (0)