Skip to content

Drop SQLite, add list mode + since filter, fix review bugs#30

Merged
cmeans merged 5 commits into
mainfrom
postgres-only
Mar 23, 2026
Merged

Drop SQLite, add list mode + since filter, fix review bugs#30
cmeans merged 5 commits into
mainfrom
postgres-only

Conversation

@cmeans
Copy link
Copy Markdown
Owner

@cmeans cmeans commented Mar 23, 2026

Summary

  • SQLite removed — PostgresStore is the only backend (~560 lines deleted). Store protocol kept as interface for future backends. All 187 tests run against real Postgres via testcontainers.
  • Lazy store initialization — no DB connection at import time, fixing review issue Add request timing and /health endpoint #7 (module-level side effects). Custom prompt sync moved to main().
  • List modeget_knowledge(mode="list") returns metadata only (id, type, source, description, tags, created, updated) without content or changelog. Also on get_alerts and get_deleted. Addresses agent UX feedback Storage abstraction, soft delete, secure deployment, README reframe #1.
  • Since filterget_knowledge(since="2026-03-23T06:00:00Z") returns only entries updated after the given timestamp, at SQL level. Also on get_alerts, get_entries, get_deleted. Addresses agent UX feedback Non-blocking cleanup, tools reference, screenshot resize #2.
  • Bug fixes — pyproject.toml version 0.1.0→0.6.0, cleanup thread accumulation guard, hardcoded Cloudflare credential UUID removed from compose
  • CI + badges — Codecov upload, README badge row (CI, coverage, Python, license, Docker)
  • Docs — CHANGELOG, README, CLAUDE.md, deployment guide, data dictionary all updated

Review findings addressed

# Finding Resolution
1 pyproject.toml version 0.1.0 Bumped to 0.6.0
2 SQLite fake pagination SQLite removed entirely
3 DateTime format inconsistency SQLite removed (was SQLite-only bug)
4 SQLite tag filtering post-fetch SQLite removed
7 Module-level store init Lazy initialization via _LazyStore
9 Cleanup thread accumulation Added is_alive() guard
Hardcoded tunnel credential UUID Replaced with required env var

QA

Prerequisites

# Install dev dependencies
pip install -e ".[dev]"

# Option A: Run automated tests (recommended — uses testcontainers, no manual Postgres setup)
# Requires Docker running (testcontainers spins up Postgres automatically)
pytest tests/ -v --cov=mcp_awareness

# Option B: Run a local server to test MCP tools manually
# Start Postgres (use the demo compose or your own)
docker compose -f docker-compose.demo.yaml up postgres -d

# Set the required env var and start the server
AWARENESS_DATABASE_URL=postgresql://awareness:mcp-awareness-demo@localhost:5432/awareness \
  mcp-awareness

Automated tests

  • 187 tests pass against real Postgres via testcontainers
  • Lint (ruff) and typecheck (mypy strict) clean
  • CI runs tests on Python 3.10, 3.11, 3.12

Manual tests (via MCP tools)

Connect to the running server, then:

    • List mode returns metadata only
    get_knowledge(mode="list", limit=5)
    

    Expected: entries with id, type, source, description, tags, created, updated — no data/content/changelog

    • Since filter returns recent entries only
    get_knowledge(since="2026-03-23T00:00:00Z", limit=5)
    

    Expected: only entries updated after the given timestamp

    • List mode + since combined
    get_knowledge(mode="list", since="2026-03-23T00:00:00Z")
    

    Expected: lightweight metadata for recent entries only

    • Alerts list mode
    get_alerts(mode="list")
    

    Expected: alert metadata without full data payloads

    • Server starts without AWARENESS_DATABASE_URL error at import
    • Source filter at SQL level
    get_knowledge(source="mcp-awareness-project", limit=3)
    

    Expected: only entries with source "mcp-awareness-project" returned (verified via SQL, not Python post-filter)

    • Empty since rejected
    get_knowledge(since="")
    

    Expected: error response — "since cannot be empty; omit or provide an ISO 8601 timestamp"

    • Alerts since filter
    get_alerts(since="2026-03-23T08:00:00Z")
    

    Expected: only alerts updated after the given timestamp
    Deploy to test instance and verify startup logs show migrations running (not import-time crash)

🤖 Generated with Claude Code

cmeans and others added 2 commits March 23, 2026 02:34
- Remove SQLiteStore (~560 lines), PostgresStore is the only backend
- Lazy store init (no DB connection at import time)
- psycopg/alembic move from optional to core dependencies
- All 187 tests run against real Postgres via testcontainers
- Add mode="list" for metadata-only queries (no content/changelog)
- Add since param for time-filtered queries (SQL-level WHERE)
- Both params on get_knowledge, get_alerts, get_deleted
- Fix cleanup thread accumulation (alive-check guard)
- Fix pyproject.toml version (0.1.0 → 0.6.0)
- Remove hardcoded Cloudflare credential UUID from compose
- Add Codecov upload to CI, badge row on README
- Update all docs: CHANGELOG, README, CLAUDE.md, deployment guide, data dictionary

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Owner Author

@cmeans cmeans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA Report — PR #30

Automated Tests

  • 187/187 tests pass (5.36s, real Postgres via testcontainers)
  • ruff: clean
  • mypy (strict): clean

Manual QA Checklist

The mode and since params are not yet deployed to production, so manual MCP tool tests can't be completed against the live server. However, the test suite covers all 5 QA items with good assertions:

# QA Item Test Coverage Automated Result
1 List mode returns metadata only test_get_knowledge_list_mode — asserts "data" not in response, checks fields present ✅ PASS
2 Since filter returns recent only test_get_knowledge_since — creates old+recent entries, asserts only recent returned ✅ PASS
3 List mode + since combined Covered by composition of 1+2 (both filter independently) ✅ PASS (implicit)
4 Alerts list mode test_get_alerts_list_mode — asserts "data" not in listing ✅ PASS
5 Server starts without import-time crash All 187 tests use lazy store via _LazyStore ✅ PASS

Code Review Findings

Bugs / Required Fixes:

  1. src/mcp_awareness/__init__.py:3__version__ = "0.1.0" should be "0.6.0" (mismatches pyproject.toml)
  2. docker-compose.yaml:6 — Image pinned to 0.5.0, should be 0.6.0 once released
  3. docker-compose.yaml:16AWARENESS_BACKEND env var is dead code (not referenced in source after SQLite removal)

Medium / Worth Noting:
4. server.py:282-283source filter on get_knowledge is Python-side, not SQL-level (pre-existing, not introduced by this PR)
5. No dedicated test for get_alerts(since=...) at tool level (store-level is tested)
6. No test for mode="list" + since combined in a single call
7. Empty string since="" silently treated as None — could confuse callers

Not Issues (Verified OK):

  • to_list_dict() correctly excludes data and changelog, includes logical_key when present
  • since filtering is SQL-level (updated >= %s) in all postgres store methods
  • Store protocol properly updated with since on all relevant methods
  • Lazy store init pattern works correctly for async/single-threaded FastMCP
  • Credential UUID properly parameterized in compose

Recommendation

Merge-ready with minor fixes — items 1-3 above should be addressed before merge. The core functionality (list mode, since filter, SQLite removal, lazy init) is solid and well-tested.


🤖 QA performed by Claude Code

@cmeans
Copy link
Copy Markdown
Owner Author

cmeans commented Mar 23, 2026

Suggested fixes for QA findings

1. __init__.py version — single source of truth
Don't update the hardcoded string. Instead, derive it from pyproject.toml so version is only tracked in one place:

from importlib.metadata import version
__version__ = version("mcp-awareness-server")

2. docker-compose.yaml:6 — image tag
Update to 0.6.0 after release (or latest if preferred).

3. docker-compose.yaml:16 — dead env var
Remove AWARENESS_BACKEND=${AWARENESS_BACKEND:-postgres} — no longer referenced in source after SQLite removal.

cmeans and others added 2 commits March 23, 2026 03:09
- __init__.py: use importlib.metadata instead of hardcoded version string
- docker-compose.yaml: remove AWARENESS_BACKEND (dead after SQLite removal)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- docker-compose.demo.yaml: remove dead AWARENESS_BACKEND env var
- CLAUDE.md: replace stale AWARENESS_DATA ref with AWARENESS_PG_DATA
- simulate_edge.py: fix --data-dir arg to --dsn for PostgresStore

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@cmeans
Copy link
Copy Markdown
Owner Author

cmeans commented Mar 23, 2026

QA Complete — All 5 Manual Tests PASS

All tests executed via live MCP tool calls against an isolated test instance (port 8421, separate awareness_qa database, claude -p --strict-mcp-config).

# Test Result
1 List mode returns metadata only ✅ PASS
2 Since filter returns recent entries only ✅ PASS
3 List mode + since combined ✅ PASS
4 Alerts list mode ✅ PASS
5 Server starts without import-time crash ✅ PASS

Automated tests

  • 187/187 pass (real Postgres via testcontainers)
  • ruff: clean
  • mypy (strict): clean

Code review findings (all addressed)

  1. __init__.py version mismatch → Fixed: now derives from importlib.metadata
  2. docker-compose.yaml image tag still 0.5.0 → Expected pre-release, update after merge+tag
  3. AWARENESS_BACKEND dead env var → Fixed: removed from both compose files

Remaining medium items (non-blocking)

  • server.py:282source filter on get_knowledge is Python-side, not SQL (pre-existing)
  • No dedicated test for get_alerts(since=...) at tool level
  • Empty string since="" silently treated as None

Ready for QA Approved label.

🤖 QA performed by Claude Code

Copy link
Copy Markdown
Owner Author

@cmeans cmeans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA Complete — All 5 Manual Tests PASS

All tests executed via live MCP tool calls against an isolated test instance (port 8421, separate awareness_qa database, claude -p --strict-mcp-config).

# Test Result
1 List mode returns metadata only ✅ PASS
2 Since filter returns recent entries only ✅ PASS
3 List mode + since combined ✅ PASS
4 Alerts list mode ✅ PASS
5 Server starts without import-time crash ✅ PASS

Automated tests

  • 187/187 pass (real Postgres via testcontainers)
  • ruff: clean
  • mypy (strict): clean

Code review findings — all addressed

  1. __init__.py version mismatch → Fixed: derives from importlib.metadata
  2. docker-compose.yaml image tag 0.5.0 → Expected pre-release
  3. AWARENESS_BACKEND dead env var → Removed

Non-blocking items for follow-up

  • server.py:282source filter on get_knowledge is Python-side (pre-existing)
  • No test for get_alerts(since=...) at tool level
  • Empty string since="" silently treated as None

🤖 QA performed by Claude Code

…tests

- Push get_knowledge source filter from Python-side to SQL-level WHERE clause
- Reject empty-string since param on get_knowledge, get_alerts, get_deleted
- Add tests: get_alerts(since=...), source SQL filter, empty since validation
- Test count: 187 → 190

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Owner Author

@cmeans cmeans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA Review — Round 2 (Full Re-review)

Automated Checks

  • 190/190 tests pass (5.87s, real Postgres via testcontainers)
  • ruff: clean
  • mypy (strict): clean

Manual QA — All 8 Tests PASS

All tests executed via live MCP tool calls against an isolated test instance (port 8421, separate database, claude -p --strict-mcp-config).

# Test Result Notes
1 List mode — metadata only ✅ PASS No data/content/changelog in response
2 Since filter — recent only ✅ PASS SQL-level updated >= %s filtering works
3 List mode + since combined ✅ PASS Both constraints applied simultaneously
4 Alerts list mode ✅ PASS Alert metadata only, no data payloads
5 Server starts cleanly ✅ PASS All tool calls succeeded, no import-time errors
6 Source filter at SQL level ✅ PASS Only matching source returned, "other-source" excluded
7 Empty since rejected ✅ PASS Returns {"error": "since cannot be empty; ..."}
8 Alerts since filter ✅ PASS Only alerts after cutoff timestamp returned

Code Review Summary

22 files reviewed across 5 commits. All prior findings (version mismatch, dead env var, source filter not at SQL level, missing since validation) have been addressed.

New finding (minor, non-blocking):

  • examples/test_new_tools.py:5 references stale AWARENESS_DATA_DIR env var removed in this PR. Either delete the file or update the usage instructions.

Observation (informational):

  • to_list_dict() returns description: "" for alerts (alerts don't have a description field in their data). Not a bug — just worth noting if list mode output is ever surfaced in UI.
  • docker-compose.yaml:6 still pins 0.5.0 — expected pre-release; update after merge+build.

Security: All SQL parametrized, no injection vectors, input validation on since, secret path auth intact, container runs as non-root.

Breaking changes: None. All new parameters are optional with backward-compatible defaults.

Verdict

PR is ready for merge. All automated and manual QA checks pass. All prior review findings addressed.

🤖 QA performed by Claude Code

@cmeans cmeans added the QA Approved Manual QA testing completed and passed label Mar 23, 2026
@cmeans cmeans merged commit 8f7e539 into main Mar 23, 2026
8 checks passed
@cmeans cmeans deleted the postgres-only branch March 23, 2026 09:08
@cmeans cmeans mentioned this pull request Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

QA Approved Manual QA testing completed and passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant