fix: Resolve CI/CD linting issues and dependency warnings#12
Closed
aaronlippold wants to merge 3 commits into
Closed
fix: Resolve CI/CD linting issues and dependency warnings#12aaronlippold wants to merge 3 commits into
aaronlippold wants to merge 3 commits into
Conversation
- Remove @functools.lru_cache from instance method to prevent memory leak (B019) - Convert _get_mime_type to static method (no state dependency) - Pin pypdfium2>=5.0.0 to avoid yanked 4.30.1 version with text extraction regression - Update uv.lock with pypdfium2 5.0.0 All 250 tests passing, ruff checks clean. Authored by: Aaron Lippold <lippold@gmail.com>
**Bug #1: Acronym Tokenization (CRITICAL)** - Problem: Search tokenizer filtered words with len <= 2 - Impact: CI, CD, UI, UX, DB, API, AWS and other acronyms were unsearchable - Fix: Changed tokenizer from 'len > 2' to 'len > 1' (src/memcord/search.py:178) - Result: All 2-letter acronyms now searchable **Bug #2: Search Index Staleness (CRITICAL)** - Problem: Search index never refreshed across MCP instances - Impact: Content saved in one session was not searchable in another session - Root Cause: Per-instance index initialized once, never checked for disk changes - Fix: Added modification time (mtime) tracking and staleness detection - Implementation: - Added _index_mtime_snapshot dict to track file mtimes - Added _is_search_index_stale() to detect changes - Updated search_memory() to check staleness before searching - Updated _save_slot() to maintain mtime snapshot on saves - Added cache invalidation when index refreshes - Result: Cross-session search now works correctly **Testing** - Added comprehensive test suite: tests/test_search_index_staleness.py - 10 tests covering same-instance, cross-instance, external modifications - All 260 tests passing (250 existing + 10 new) - Performance validated: staleness check adds < 50ms overhead **Impact** Fixes core search functionality that was completely broken for: - Cross-session workflows - Multi-instance collaboration - External modifications - Common acronym searches (CI/CD, API, UI, DB, AWS, etc.) Authored by: Aaron Lippold <lippold@gmail.com>
**Coverage**: 22 end-to-end integration tests across 4 test classes **Test Classes**: 1. TestBasicWorkflows (6 tests) - Save and search workflow - Acronym searches (CI/CD, API, UI, DB, AWS) - Cross-session discovery - Tag filtering - List and read workflows 2. TestConcurrentWorkflows (2 tests) - Concurrent multi-session saves - Update and search workflows 3. TestErrorRecoveryWorkflows (4 tests) - Empty database graceful handling - Empty content validation - Nonexistent slot handling - Tag operations workflow 4. TestRealWorldScenarios (3 tests) - Multi-project context switching - Decision tracking over time - Long-running project sessions 5. TestCommandInteractions (4 tests) - List tags after tagging - Slot switching and reading - Save overwrite behavior (manual_save replaces) - Save progress append behavior (auto_summary appends) 6. TestEdgeCasesAndRegression (3 tests) - Special characters in search - Case-insensitive search - Unicode content handling **Why These Tests Matter**: - Would have caught search index staleness bug - Would have caught acronym tokenization bug - Test from user perspective, not implementation details - Cover cross-instance scenarios (realistic usage) - Validate real-world workflows, not just individual commands **Results**: All 282 tests passing (250 existing + 10 staleness + 22 E2E) Authored by: Aaron Lippold <lippold@gmail.com>
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
1 participant
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.
Summary
Comprehensive CI/CD fixes and search functionality improvements with extensive testing.
Changes
🐛 Bug Fixes
Fix #1: Ruff B019 Linting Error
@functools.lru_cacheon instance method can cause memory leakssrc/memcord/server.py:906_get_mime_typeto@staticmethodFix #2: Yanked Dependency
pypdfium2==4.30.1yanked due to text extraction regressionpypdfium2>=5.0.0in pyproject.tomlFix #3: Acronym Search Support
len > 2tolen > 1in search.py:178Fix #4: Cross-Session Search
_index_mtime_snapshot_is_search_index_stale()✅ Testing Improvements
New Test Suites
1. Search Index Tests (
test_search_index_staleness.py- 10 tests)2. E2E Workflow Tests (
test_e2e_workflows.py- 22 tests)Testing Philosophy:
📊 Test Results
Test Count: 250 → 282 tests (+32 new tests, +12.8%)
CI/CD Status:
ruff check .cleanruff format --check .clean (43 files)📈 Impact
Improvements:
Files Changed: 7 total
🧪 Verification
uv sync --group dev uv run pytest tests/ --tb=no -q # Expected: 282 passedAll tests passing, ready for review!