Commit graph

13 commits

Author SHA1 Message Date
BukeLy
5180c1e395 feat: implement dimension compatibility checks for PostgreSQL and Qdrant migrations
This update introduces checks for vector dimension compatibility before migrating legacy data in both PostgreSQL and Qdrant storage implementations. If a dimension mismatch is detected, the migration is skipped to prevent data loss, and a new empty table or collection is created for the new embedding model.

Key changes include:
- Added dimension checks in `PGVectorStorage` and `QdrantVectorDBStorage` classes.
- Enhanced logging to inform users about dimension mismatches and the creation of new storage.
- Updated E2E tests to validate the new behavior, ensuring legacy data is preserved and new structures are created correctly.

Impact:
- Prevents potential data corruption during migrations with mismatched dimensions.
- Improves user experience by providing clear logging and maintaining legacy data integrity.

Testing:
- New tests confirm that the system behaves as expected when encountering dimension mismatches.
2025-11-20 12:22:13 +08:00
BukeLy
e0767b1a47 fix: correct Qdrant point ID type in dimension mismatch E2E test
Why this change is needed:
The test was failing not due to dimension mismatch logic, but because
of invalid point ID format. Qdrant requires point IDs to be either
unsigned integers or UUIDs.

How it solves it:
Changed from id=str(i) (which produces "0", "1", "2" - invalid) to
id=i (which produces 0, 1, 2 - valid unsigned integers).

Impact:
- Fixes false test failure caused by test code bug
- Now test will properly verify actual dimension mismatch handling
- Aligned with other E2E tests that use integer IDs

Testing:
Will verify on CI that test now runs to completion and checks real
dimension mismatch behavior (not test setup errors)
2025-11-20 12:13:58 +08:00
BukeLy
e1e1080edf test: add E2E tests for dimension mismatch scenarios
Why this change is needed:
Codex review identified two P1 bugs where vector dimension mismatches during
migration cause startup failures. Current tests only validate same-dimension
migrations (e.g., 1536d->1536d), missing the upgrade scenario (e.g., 1536d->3072d).
These new tests expose the gaps in existing migration logic.

How it solves it:
Added two E2E tests to test_e2e_multi_instance.py:
- test_dimension_mismatch_postgres: 1536d -> 3072d upgrade scenario
- test_dimension_mismatch_qdrant: 768d -> 1024d upgrade scenario

Both tests create legacy collections/tables with old dimension vectors, then
attempt to initialize with new dimension models. Tests verify either graceful
handling (create new storage for new model) or clear error messages.

Impact:
- Exposes dimension mismatch bugs in migration logic
- Tests will fail until migration logic is fixed
- Provides safety net for future dimension changes
- Documents expected behavior for model upgrades

Testing:
These tests are expected to FAIL in CI, demonstrating the P1 bugs exist.
Once migration logic is fixed to handle dimension mismatches, tests will pass.
2025-11-20 12:07:31 +08:00
BukeLy
8386ea061e refactor: unify PostgreSQL and Qdrant migration logic for consistency
Why this change is needed:
Previously, PostgreSQL and Qdrant had inconsistent migration behavior:
- PostgreSQL kept legacy tables after migration, requiring manual cleanup
- Qdrant auto-deleted legacy collections after migration
This inconsistency caused confusion for users and required different
documentation for each backend.

How it solves the problem:
Unified both backends to follow the same smart cleanup strategy:
- Case 1 (both exist): Auto-delete if legacy is empty, warn if has data
- Case 4 (migration): Auto-delete legacy after successful verification
This provides a fully automated migration experience without manual intervention.

Impact:
- Eliminates need for users to manually delete legacy tables/collections
- Reduces storage waste from duplicate data
- Provides consistent behavior across PostgreSQL and Qdrant
- Simplifies documentation and user experience

Testing:
- All 16 unit tests pass (8 PostgreSQL + 8 Qdrant)
- Added 4 new tests for Case 1 scenarios (empty vs non-empty legacy)
- Updated E2E tests to verify auto-deletion behavior
- All lint checks pass (ruff-format, ruff, trailing-whitespace)
2025-11-20 11:37:59 +08:00
BukeLy
cedb3d49d2 fix: pass workspace to LightRAG instance instead of vector_db_storage_cls_kwargs
Why this change is needed:
LightRAG creates storage instances by passing its own self.workspace field,
not the workspace parameter from vector_db_storage_cls_kwargs. This caused
E2E tests to fail because the workspace was set to default "_" instead of
the configured value like "prod" or "workspace_a".

How it solves it:
- Pass workspace directly to LightRAG constructor as a field parameter
- Remove workspace from vector_db_storage_cls_kwargs where it was being ignored
- This ensures self.workspace is set correctly and propagated to storage instances

Impact:
- Fixes test_backward_compat_old_workspace_naming_qdrant migration failure
- Fixes test_workspace_isolation_e2e_qdrant workspace mismatch
- Proper workspace isolation is now enforced in E2E tests

Testing:
- Modified two Qdrant E2E tests to use correct workspace configuration
- Tests should now find correct legacy collections (e.g., prod_chunks)
2025-11-20 03:09:46 +08:00
BukeLy
0508ad7a15 fix: prevent offline tests from failing due to missing E2E dependencies
Why this change is needed:
Offline tests were failing with "ModuleNotFoundError: No module named 'qdrant_client'"
because test_e2e_multi_instance.py was being imported during test collection, even
though it's an E2E test that shouldn't run in offline mode. Pytest imports all test
files during collection phase regardless of marks, causing import errors for missing
E2E dependencies (qdrant_client, asyncpg, etc.).

Additionally, the test mocks for PostgreSQL migration were too permissive - they
accepted any parameter format without validation, which allowed bugs (like passing
dict instead of positional args to AsyncPG execute()) to slip through undetected.

How it solves it:
1. E2E Import Fix:
   - Use pytest.importorskip() to conditionally import qdrant_client
   - E2E tests are now skipped cleanly when dependencies are missing
   - Offline tests can collect and run without E2E dependencies

2. Stricter Test Mocks:
   - Enhanced mock_pg_db fixture to validate AsyncPG parameter format
   - Mock execute() now raises TypeError if dict/list passed as single argument
   - Ensures tests catch parameter passing bugs that would fail in production

3. Parameter Validation Test:
   - Added test_postgres_migration_params.py for explicit parameter validation
   - Verifies migration passes positional args correctly to AsyncPG
   - Provides detailed output for debugging parameter issues

Impact:
- Offline tests no longer fail due to missing E2E dependencies
- Future bugs in AsyncPG parameter passing will be caught by tests
- Better test isolation between offline and E2E test suites
- Improved test coverage for migration parameter handling

Testing:
- Verified with `pytest tests/ -m offline -v` - no import errors
- All PostgreSQL migration tests pass (6/6 unit + 1 strict validation)
- Pre-commit hooks pass (ruff-format, ruff)
2025-11-20 02:03:48 +08:00
BukeLy
19caf9f27c test: add comprehensive E2E migration tests for Qdrant and complete unit test coverage
Why this change is needed:
The previous test coverage had gaps in critical migration scenarios that could lead
to data loss or broken upgrades for users migrating from old versions of LightRAG.

What was added:

1. E2E Tests (test_e2e_multi_instance.py):
   - test_case1_both_exist_warning_qdrant: Verify warning when both collections exist
   - test_case2_only_new_exists_qdrant: Verify existing collection reuse
   - test_backward_compat_old_workspace_naming_qdrant: Test old workspace naming migration
   - test_empty_legacy_qdrant: Verify empty legacy collection handling
   - test_workspace_isolation_e2e_qdrant: Validate workspace data isolation

2. Unit Tests (test_migration_complete.py):
   - All 4 migration cases (new+legacy, only new, only legacy, neither)
   - Backward compatibility tests for multiple legacy naming patterns
   - Empty legacy migration scenario
   - Workspace isolation verification
   - Model switching scenario
   - Full migration lifecycle integration test

How it solves it:
These tests validate the _find_legacy_collection() backward compatibility fix with
real Qdrant database instances, ensuring smooth upgrades from all legacy versions.

Impact:
- Prevents regressions in migration logic
- Validates backward compatibility with old naming schemes
- Ensures workspace isolation works correctly
- Will run in CI pipeline to catch issues early

Testing:
All 20+ tests pass locally. E2E tests will validate against real Qdrant in CI.
2025-11-20 01:47:09 +08:00
BukeLy
65ff9b32bd style: fix lint errors in E2E test file
Remove unused embedding functions (C and D) that were defined but never
used, causing F841 lint errors.

Also fix E712 errors by using 'is True' instead of '== True' for
boolean comparisons in assertions.

Testing:
- All pre-commit hooks pass
- Verified with: uv run pre-commit run --all-files
2025-11-20 01:32:42 +08:00
BukeLy
e9f6cedff8 fix: use NetworkXStorage for E2E tests (AGE extension not available in CI)
Why this change is needed:
E2E PostgreSQL tests were failing because they specified graph_storage="PGGraphStorage",
but the CI environment doesn't have the Apache AGE extension installed. This caused
initialize_storages() to fail with "function create_graph(unknown) does not exist".

How it solves it:
Removed graph_storage="PGGraphStorage" parameter in all PostgreSQL E2E tests,
allowing LightRAG to use the default NetworkXStorage which doesn't require
external dependencies.

Impact:
- PostgreSQL E2E tests can now run successfully in CI
- Vector storage migration tests can complete without AGE extension dependency
- Maintains test coverage for vector storage model isolation feature

Testing:
The vector storage migration tests (which are the focus of this PR) don't
depend on graph storage implementation and can run with NetworkXStorage.
2025-11-20 01:15:20 +08:00
BukeLy
bf176b38ee fix: correct attribute access in E2E tests
Why this change is needed:
Tests were accessing rag.chunk_entity_relation_graph.chunk_vdb which
doesn't exist. The chunk_entity_relation_graph is a BaseGraphStorage
and doesn't have a chunk_vdb attribute.

How it solves it:
Changed all occurrences to use direct LightRAG attributes:
- rag.chunks_vdb.table_name (PostgreSQL)
- rag.chunks_vdb.final_namespace (Qdrant)

Impact:
Fixes AttributeError that would occur when E2E tests run

Testing:
Will verify on GitHub Actions E2E test run
2025-11-20 00:47:16 +08:00
BukeLy
38f41daa3d fix: remove non-existent storage kwargs in E2E tests
Why this change is needed:
E2E tests were failing with TypeError because they used non-existent
parameters kv_storage_cls_kwargs, graph_storage_cls_kwargs, and
doc_status_storage_cls_kwargs. These parameters do not exist in
LightRAG's __init__ method.

How it solves it:
Removed the three non-existent parameters from all LightRAG initializations
in test_e2e_multi_instance.py:
- test_legacy_migration_postgres
- test_multi_instance_postgres (both instances A and B)

PostgreSQL storage classes (PGKVStorage, PGGraphStorage, PGDocStatusStorage)
use ClientManager which reads configuration from environment variables
(POSTGRES_HOST, POSTGRES_PORT, etc.) that are already set in the E2E
workflow, so no additional kwargs are needed.

Impact:
- Fixes TypeError on LightRAG initialization
- E2E tests can now properly instantiate with PostgreSQL storages
- Configuration still works via environment variables

Testing:
Next E2E run should successfully initialize LightRAG instances
and proceed to actual migration/multi-instance testing.
2025-11-20 00:32:16 +08:00
BukeLy
c7e7b347e9 test: add Qdrant legacy migration E2E test
Why this change is needed:
Complete E2E test coverage for vector model isolation feature requires
testing legacy data migration for both PostgreSQL and Qdrant backends.
Previously only PostgreSQL migration was tested.

How it solves it:
- Add test_legacy_migration_qdrant() function to test automatic migration
  from legacy collection (no model suffix) to model-suffixed collection
- Test creates legacy "lightrag_vdb_chunks" collection with 1536d vectors
- Initializes LightRAG with model_name="text-embedding-ada-002"
- Verifies automatic migration to "lightrag_vdb_chunks_text_embedding_ada_002_1536d"
- Validates vector count, dimension, and collection existence

Impact:
- Ensures Qdrant migration works correctly in real scenarios
- Provides parity with PostgreSQL E2E test coverage
- Will be automatically run in CI via -k "qdrant" filter

Testing:
- Test follows same pattern as test_legacy_migration_postgres
- Uses complete LightRAG initialization with mock LLM and embedding
- Includes proper cleanup via qdrant_cleanup fixture
- Syntax validated with python3 -m py_compile
2025-11-20 00:19:21 +08:00
BukeLy
dc2061583f test: refactor E2E tests using complete LightRAG instances
Replaced storage-level E2E tests with comprehensive LightRAG-based tests.

Key improvements:
- Use complete LightRAG initialization (not just storage classes)
- Proper mock LLM/embedding functions matching real usage patterns
- Added tokenizer support for realistic testing

Test coverage:
1. test_legacy_migration_postgres: Automatic migration from legacy table (1536d)
2. test_multi_instance_postgres: Multiple LightRAG instances (768d + 1024d)
3. test_multi_instance_qdrant: Multiple Qdrant instances (768d + 1024d)

Scenarios tested:
- ✓ Multi-dimension support (768d, 1024d, 1536d)
- ✓ Multi-model names (model-a, model-b, text-embedding-ada-002)
- ✓ Legacy migration (backward compatibility)
- ✓ Multi-instance coexistence
- ✓ PostgreSQL and Qdrant storage backends

Removed:
- tests/test_e2e_postgres_migration.py (replaced)
- tests/test_e2e_qdrant_migration.py (replaced)

Updated:
- .github/workflows/e2e-tests.yml: Use unified test file
2025-11-20 00:13:00 +08:00