Address code review feedback: extract magic numbers and fix variable reassignment bug#15
Merged
royisme merged 3 commits intoclaude/auto-extract-memories-011CUq28jWKZ2GA714gTGsj2from Nov 5, 2025
Conversation
Contributor
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ble reassignment, update test count Co-authored-by: royisme <350731+royisme@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot
AI
changed the title
[WIP] Implement automatic memory extraction features
Address code review feedback: extract magic numbers and fix variable reassignment bug
Nov 5, 2025
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
royisme
approved these changes
Nov 5, 2025
royisme
merged commit ec5315f
into
claude/auto-extract-memories-011CUq28jWKZ2GA714gTGsj2
royisme
pushed a commit
that referenced
this pull request
Nov 6, 2025
…n system
BREAKING CHANGE: Default docker-compose.yml now points to minimal mode
## Docker Infrastructure
### Multi-Mode Deployment
- **Minimal**: Code Graph only (No LLM required) - 500MB image
- **Standard**: Code Graph + Memory (Embedding required) - 600MB image
- **Full**: All features (LLM + Embedding) - 800MB image
### Files Added
- docker/Dockerfile.{base,minimal,standard,full}
- docker/docker-compose.{minimal,standard,full}.yml
- docker/.env.template/.env.{minimal,standard,full}
- docker-compose.yml (default, points to minimal)
### Automation
- Makefile with convenience commands (docker-minimal, docker-standard, docker-full)
- scripts/docker-deploy.sh - Interactive deployment wizard
- GitHub Actions for automated Docker builds (royisme/codebase-rag)
- Multi-arch support (AMD64, ARM64)
## Documentation System
### MkDocs Material
- Configured for docs.vantagecraft.dev
- English-first documentation
- Dark mode support
- Search, code highlighting, Mermaid diagrams
### Documentation Pages
- index.md - Homepage with feature comparison table
- getting-started/quickstart.md - 5-minute quick start guide
- deployment/overview.md - Comprehensive mode comparison
- deployment/production.md - Production deployment (K8s, Docker Swarm, Nginx)
### CI/CD
- .github/workflows/docs-deploy.yml - Auto-deploy to GitHub Pages
- .github/workflows/docker-build.yml - Auto-build Docker images
- docs/CNAME - Domain configuration
## Features by Mode
| Feature | Minimal | Standard | Full |
|---------|---------|----------|------|
| Code Graph | ✅ | ✅ | ✅ |
| Memory Store | ❌ | ✅ | ✅ |
| Auto Extraction | ❌ | ❌ | ✅ |
| Knowledge RAG | ❌ | ❌ | ✅ |
| LLM Required | ❌ | ❌ | ✅ |
| Embedding Required | ❌ | ✅ | ✅ |
## Quick Start
```bash
# Minimal deployment (no LLM needed)
make docker-minimal
# Standard deployment (embedding needed)
make docker-standard
# Full deployment (LLM + embedding needed)
make docker-full
```
## Next Steps
Code changes required for dynamic mode switching:
- config.py: Add DeploymentMode enum and validation
- start_mcp.py: Add --mode argument parsing
- mcp_server.py: Dynamic tool registration based on mode
See DOCKER_IMPLEMENTATION_SUMMARY.md for details.
## Documentation
Will be available at: https://docs.vantagecraft.dev
Related: #14, #15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses code review comments from PR #14, fixing a variable reassignment bug, potential IndexError, and extracting hard-coded limits to named constants. Also updates test to reflect correct tool count (30 instead of 25).
Bug Fixes
Variable reassignment causing potential AttributeError:
IndexError for files without extensions:
Code Quality Improvements
Extracted 7 magic numbers to class-level constants in
MemoryExtractor:MAX_COMMITS_TO_PROCESS = 20- batch processing limitMAX_FILES_TO_SAMPLE = 30- file scanning limitMAX_ITEMS_PER_TYPE = 3- top results per memory typeMAX_README_LINES = 20- documentation parsing limitMAX_STRING_EXCERPT_LENGTH = 200- log/response excerptsMAX_CONTENT_LENGTH = 500- memory content field limitMAX_TITLE_LENGTH = 100- memory title field limitTest Updates
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.