1001-migrate-memu-server by SparkZou · Pull Request #11 · NevaMind-AI/memU-server

SparkZou · 2026-01-21T04:55:36Z

🎯 Summary

Complete migration to memu-server with full development setup and tooling.

📝 Changes

✅ Setup project structure and gitignore
✅ Add dependencies and development environment (FastAPI, PostgreSQL, Temporal)
✅ Configure PostgreSQL (port 54320) and Temporal (port 17233) with custom ports
✅ Add database models (Memory, MemoryCategory, MemorizeTask)
✅ Implement Alembic database migrations
✅ Add comprehensive testing setup with pytest
✅ Setup code quality tools (ruff, pre-commit, bandit)
✅ Add Makefile for common development tasks
✅ Setup GitHub Actions CI/CD pipeline
✅ Add environment variable validation
✅ Create comprehensive English README with documentation

🧪 Testing

All tests passing ✅

Code formatting checked with ruff
Linting passed with ruff check
Security scan passed with bandit
Pre-commit hooks configured and working

📚 Documentation

Complete README with setup instructions
Code quality guidelines
Pre-commit hooks usage
Development workflow documented

✅ Checklist

- Setup project structure and gitignore - Add dependencies and development environment - Configure PostgreSQL and Temporal with custom ports - Add database models (Memory, MemoryCategory, MemorizeTask) - Implement Alembic migrations - Add comprehensive testing setup with pytest - Setup code quality tools (ruff, pre-commit, bandit) - Add Makefile for common development tasks - Setup GitHub Actions CI/CD pipeline - Add environment variable validation - Create comprehensive English README with documentation Co-authored-by: GitHub Copilot <noreply@github.com>

Copilot

Pull request overview

This PR performs a comprehensive migration to memu-server with extensive infrastructure setup, development tooling, and CI/CD configuration. The migration transitions from Python 3.14 (non-existent) to Python 3.13, adds PostgreSQL and Temporal integration, and establishes a complete development workflow with testing, linting, and pre-commit hooks.

Changes:

Migrated Python version from 3.14 to 3.13 and updated all dependencies
Added infrastructure services (PostgreSQL with pgvector, Temporal) via Docker Compose on custom ports
Implemented comprehensive development tooling (Makefile, pre-commit hooks, GitHub Actions CI/CD)
Enhanced application configuration with environment validation and database integration
Added basic test setup and documentation

Reviewed changes

Copilot reviewed 15 out of 28 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
pyproject.toml	Updated dependencies, added dev tools (pytest, ruff, bandit), switched to dependency-groups
.python-version	Corrected from 3.14 to 3.13
docker-compose.yml	Added PostgreSQL (port 54320) and Temporal (ports 17233, 18233) services
app/main.py	Enhanced with environment validation, database config, improved error handling
app/database.py	Added async database session management with SQLAlchemy
alembic.ini	Added Alembic configuration for database migrations
alembic/env.py	Setup migration environment with database imports
alembic/versions/250177434a27_initial_migration.py	Added empty initial migration
tests/test_health.py	Added basic health check test with environment setup
.github/workflows/code-quality.yml	Added CI/CD pipeline for formatting, linting, and testing
.pre-commit-config.yaml	Added pre-commit hooks for code quality and security checks
Makefile	Added development commands for common tasks
.env.example	Added comprehensive environment configuration template
README.md	Complete rewrite with installation guide, architecture, and development workflow
.gitignore	Streamlined for Python project with database and storage exclusions
GitHub issue templates	Fixed trailing whitespace

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-21T05:01:15Z

+        "url": os.getenv(
+            "DATABASE_URL",
+            "postgresql+psycopg://memu_user:memu_pass@localhost:54320/memu_db",
+        )


The database password is exposed in the default DATABASE_URL string. While this is a fallback for development environments, having credentials directly in the code poses a security risk. If this code is deployed without proper environment variables set, the default credentials would be used. Consider either removing the fallback entirely to force explicit configuration, or at a minimum, add a warning log when using default credentials. Additionally, document clearly that these defaults are for local development only.

- Remove hardcoded database credentials fallback values - Add runtime validation for required environment variables - Add pylint and mypy for enhanced code quality checks - Improve exception handling with explicit raise from - Fix import order and line length issues - Pylint score: 9.83/10 This addresses GitHub Copilot security recommendations.

- Add custom pre-commit hook to detect empty Alembic migrations - Remove empty initial migration file - Add 'make migrate-check' command for manual validation - Auto-check migrations after 'make migrate-create' This addresses GitHub Copilot feedback about empty migration files. Empty migrations indicate models weren't properly imported when running autogenerate, helping catch configuration issues early.

- Add pre-commit hook to detect hardcoded credentials in config files - Check .ini, .yaml, .toml files for database URLs, API keys, passwords - Fix alembic.ini to use environment variable placeholders - Use ${DATABASE_USER} format as recommended by GitHub Copilot This addresses the hardcoded credential issue in alembic.ini that wasn't caught by Python-only tools like bandit.

- Configure ruff with isort (I001) for automatic import ordering - Enable pyupgrade (UP) to use Python 3.13+ type annotations - Replace typing.Dict with built-in dict - Auto-fix all import order violations in codebase - Add pylint ignores for known false positives - All quality checks passing (pytest, pylint, ruff)

- Add test coverage validation pre-commit hook (35% minimum) - Create tests for OPENAI_API_KEY validation logic * Test app refuses to start without API key * Test app refuses empty API key * Test app starts with valid API key - Add 'make test-cov' command for coverage reports - Test coverage increased: 36% → 38% - Now covers the validation logic Copilot suggested testing

- Replace 'uv pip install -e ".[dev]"' with 'uv sync' - Update Makefile: install → uv sync --no-dev, dev → uv sync - Update GitHub Actions workflow to use uv sync - Add pre-commit hook to detect deprecated uv commands - Fixes Copilot suggestions in PR review

- Use ${VAR_NAME:-default} syntax for DB credentials - Enhance credential scanner to detect YAML-style patterns - Add POSTGRES_* variables to .env.example - Now detects hardcoded credentials in docker-compose.yml - Fixes Copilot security suggestion in PR review

- Replace all escaped backticks with proper markdown - Remove duplicate Code Quality section - Add markdown formatting checker to pre-commit hooks - Fixes Copilot markdown suggestions

- Require DATABASE_URL environment variable at startup - Add runtime validation with clear error message - Add test for DATABASE_URL requirement - Create Python credential scanner for pre-commit hooks - Detects database URLs with embedded credentials - Fixes Copilot security suggestion in app/main.py

Copilot

Pull request overview

Copilot reviewed 21 out of 34 changed files in this pull request and generated 17 comments.

Comments suppressed due to low confidence (1)

app/main.py:52

The memorize endpoint saves conversation files to disk (lines 50-52) but doesn't handle potential I/O errors separately from the general exception handling. If the storage directory is not writable or disk is full, users will get a generic 500 error. Consider adding specific error handling for filesystem operations with a more descriptive error message.

        file_path = storage_dir / f"conversation-{uuid.uuid4().hex}.json"
        with file_path.open("w", encoding="utf-8") as f:
            json.dump(payload, f, ensure_ascii=False)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-21T21:03:33Z

+# Run tests with coverage
+test-cov:
+	@echo "🔍 Running tests with coverage..."
+	@pytest --cov=app --cov-report=term-missing --cov-report=html --cov-fail-under=80 tests/


The test-cov target has a coverage threshold of 80% (line 38) but the pre-commit hook uses 35%. This inconsistency means that 'make test-cov' will fail while pre-commit passes. Both should use the same threshold for consistency, or there should be documentation explaining why they differ.

Copilot · 2026-01-21T21:03:34Z

+- One call = response + memory 👉 memU Response API: https://memu.pro/docs#responseapi
+- Try it instantly 👉 https://app.memu.so/quick-start

 ---


The horizontal rule separators ('----') between sections have been removed throughout the README. While this is a stylistic choice, it makes the document structure less visually distinct. The old README had clear section separators that improved readability. Consider whether this was intentional or if some visual separation between major sections would be beneficial.

Copilot · 2026-01-21T21:03:36Z

+            f"Database configuration is incomplete. Missing environment variables: {', '.join(missing_vars)}"
+        )
+
+    DATABASE_URL = f"postgresql+psycopg://{db_user}:{db_pass}@{db_host}:{db_port}/{db_name}"


The database connection string construction (line 38) uses psycopg without async driver specification. The DATABASE_URL examples show 'postgresql+psycopg://' but this might not be compatible with the async engine created on line 41. For async operations with psycopg3, you should use 'postgresql+psycopg' or explicitly configure psycopg in async mode. Verify that this works correctly with async_engine.

Suggested change

DATABASE_URL = f"postgresql+psycopg://{db_user}:{db_pass}@{db_host}:{db_port}/{db_name}"

DATABASE_URL = f"postgresql+asyncpg://{db_user}:{db_pass}@{db_host}:{db_port}/{db_name}"

Copilot · 2026-01-21T21:03:36Z

+      run: |
+        uv run ruff check .
+
+    - name: Run tests


The CI workflow runs tests (line 43) but doesn't set up the required environment variables (OPENAI_API_KEY, DATABASE_URL) that are validated in app/main.py. The tests will fail during CI because the app module import will raise a RuntimeError when these environment variables are missing. Add environment variable configuration to the workflow or mock them in the test setup.

Suggested change

- name: Run tests

- name: Run tests

env:

OPENAI_API_KEY: "test-openai-key"

DATABASE_URL: "sqlite:///:memory:"

Copilot · 2026-01-21T21:03:37Z

+# Database
+DATABASE_HOST=localhost
+DATABASE_PORT=54320
+DATABASE_USER=memu_user
+DATABASE_PASSWORD=memu_pass
+DATABASE_NAME=memu_db


The README shows 'DATABASE_URL' as a single connection string option (lines 23, 105-109) but doesn't mention that it takes priority over the individual DATABASE_* variables according to app/database.py. The configuration section should clarify that DATABASE_URL can be used as an alternative to setting individual database variables, and which one takes precedence.

Copilot · 2026-01-21T21:03:37Z

+│   ├── database.py   # Database configuration
+│   └── main.py       # FastAPI application
+├── config/
+│   └── settings.py   # Configuration management


The project structure documentation (lines 271-280) mentions 'config/settings.py' but this file doesn't exist in the PR changes. The config directory only contains an empty init.py. Either this file needs to be added or the documentation should be updated to reflect the actual structure.

Suggested change

│ └── settings.py # Configuration management

│ └── __init__.py # Configuration package

Copilot · 2026-01-21T21:03:37Z

+    database_config={"url": database_url},
+)
+
+storage_dir = Path(os.getenv("STORAGE_PATH", "./data"))


Environment variable name inconsistency: The code uses 'STORAGE_PATH' (line 43) but the old code used 'MEMU_STORAGE_DIR'. This breaking change should be documented in the migration guide or PR description. Additionally, the .env.example file sets STORAGE_PATH to '/var/data/memu-server' but the code defaults to './data', which could cause confusion.

Suggested change

storage_dir = Path(os.getenv("STORAGE_PATH", "./data"))

storage_path = os.getenv("STORAGE_PATH") or os.getenv("MEMU_STORAGE_DIR") or "/var/data/memu-server"

storage_dir = Path(storage_path)

SparkZou and others added 2 commits January 21, 2026 13:44

fix: correct GitHub Actions workflow to use uv run

f909115

Copilot AI review requested due to automatic review settings January 21, 2026 04:55

Copilot started reviewing on behalf of SparkZou January 21, 2026 04:56 View session

SparkZou changed the title ~~feat: migrate to memu-server with complete setup~~ 1001-migrate-memu-server Jan 21, 2026

Copilot AI reviewed Jan 21, 2026

View reviewed changes

SparkZou added 9 commits January 22, 2026 04:29

fix: correct markdown code fence formatting in README

8b02d16

- Replace all escaped backticks with proper markdown - Remove duplicate Code Quality section - Add markdown formatting checker to pre-commit hooks - Fixes Copilot markdown suggestions

Copilot AI review requested due to automatic review settings January 21, 2026 20:54

Copilot started reviewing on behalf of SparkZou January 21, 2026 20:55 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

SparkZou closed this Jan 21, 2026

SparkZou deleted the feature/1001-migrate-memu-server branch January 21, 2026 21:25

	DATABASE_URL = f"postgresql+psycopg://{db_user}:{db_pass}@{db_host}:{db_port}/{db_name}"
	DATABASE_URL = f"postgresql+asyncpg://{db_user}:{db_pass}@{db_host}:{db_port}/{db_name}"

	│ └── settings.py # Configuration management
	│ └── __init__.py # Configuration package

	storage_dir = Path(os.getenv("STORAGE_PATH", "./data"))
	storage_path = os.getenv("STORAGE_PATH") or os.getenv("MEMU_STORAGE_DIR") or "/var/data/memu-server"
	storage_dir = Path(storage_path)

Conversation

SparkZou commented Jan 21, 2026

🎯 Summary

📝 Changes

🧪 Testing

📚 Documentation

✅ Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants