docs(rag): final report on dim-mismatch recursion fix

2026-06-09 15:04:42 -04:00
parent 40f905d14b
commit b4d240a9f3
1 changed files with 78 additions and 0 deletions
@@ -0,0 +1,78 @@
+# RAG Work Final Report (2026-06-09 PM)
+
+## TL;DR
+
+**RAG unit tests PASS in batch (5/5).** RAG test in `live_gui` batch: `test_rag_phase4_final_verify` PASSES (7.56s). A SEPARATE stress test (`test_rag_phase4_stress`) fails in batch on a pre-existing io_pool race in `app_controller.py:_sync_rag_engine` — explicitly out of scope for the wipe fix per the prior report. The dim-mismatch bug that started this whole thread is **fixed and verified in batch**.
+
+## What Shipped This Session
+
+Two atomic commits:
+
+1. **`644d88ab` — `fix(rag): break recursion in _validate_collection_dim`**
+   - Wipe path called `self._init_vector_store()` which re-invoked `_validate_collection_dim`, causing infinite recursion (`RecursionError`) on dim mismatch with the mock embedding provider.
+   - Fix: re-initialize the vector store INLINE after the `rmtree` wipe so the fresh collection is created without re-validating.
+
+2. **`40f905d1` — `test(rag): update dim-mismatch test to assert rmtree behavior`**
+   - Test was asserting the old `client.delete_collection` contract.
+   - Updated to assert the new rmtree behavior (no delete_collection call, 2 get_or_create_collection calls).
+
+## Verification
+
+| Test | In isolation | In batch (focused live_gui run) |
+|------|-------------|----------------------------------|
+| `test_rag_collection_dim_mismatch_recreates_collection` | PASS (0.10s) | PASS (0.10s) |
+| `test_rag_collection_dim_match_preserves_collection` | PASS (0.10s) | PASS (0.10s) |
+| `test_rag_engine_init_mock` | PASS | PASS |
+| `test_rag_engine_chroma` | PASS | PASS |
+| `test_local_embedding_provider_missing_dependency_has_install_hint` | PASS | PASS |
+| `test_rag_phase4_final_verify` | PASS (7.56s) | PASS (~5s) |
+| `test_rag_visual_sim` | PASS | PASS (2 tests) |
+| `test_rag_phase4_stress` | PASS (per `rag_test_fix_final_20260609.md`) | **FAIL** (out of scope bug) |
+
+**Full tier-1 batch (`uv run .\scripts\run_tests_batched.py`):**
+- tier-1-unit-comms: PASS (30.6s)
+- tier-1-unit-core: PASS (64.2s) ← includes all rag_engine tests
+- tier-1-unit-gui: PASS (26.8s)
+- tier-1-unit-headless: PASS (23.6s)
+- tier-1-unit-mma: PASS (26.3s)
+- tier-2-mock_app-comms: PASS (7.8s)
+- tier-2-mock_app-core: PASS (12.5s)
+- tier-2-mock_app-gui: PASS (10.7s)
+- tier-2-mock_app-headless: PASS (8.7s)
+- tier-2-mock_app-mma: PASS (12.1s)
+- tier-3-live_gui: **FAIL** at `test_gui2_set_value_hook_works` line 41 (unrelated, pre-existing, see "Out of Scope" below)
+
+**RAG-focused live_gui batch (workaround for the parity test):**
+- `test_rag_phase4_final_verify` PASS
+- `test_rag_phase4_stress` FAIL
+- `test_rag_visual_sim` (2 tests) PASS
+- Total: 3 PASS, 1 FAIL in 30.34s
+
+## What's NOT Fixed (Out of Scope)
+
+1. **`test_rag_phase4_stress` batch failure** — The "Modified context not found in discussion" failure is the io_pool race in `app_controller.py:_sync_rag_engine` documented in the prior report (`f207d297`). Multiple setters in quick succession (`rag_collection_name`, `files`, `rag_enabled`, `rag_source`, `rag_emb_provider`) submit parallel sync tasks, last-finished-wins, indexing is non-deterministic. **The proper fix is a debounce/coalescing pattern in the controller** — multi-file refactor, belongs in a track per the foundation document `test_infra_hardening_foundation_20260608.md`.
+
+2. **`test_gui2_set_value_hook_works` batch failure** — `set_value` hook returns `'queued'` but `get_value('ai_input')` returns `''` after 1.5s. Different code path from RAG, pre-existing, not investigated this session per the Deduction Loop rule (2-failure cap). Likely a `setattr` routing issue in `gui_2.py` (same class of bug as the earlier `_UI_FLAG_DEFAULTS` fix). Out of scope for the RAG fix.
+
+## Net Result
+
+- **Dim-mismatch recursion bug: FIXED.** No more `RecursionError` on collection dim mismatch.
+- **RAG test (`test_rag_phase4_final_verify`): PASSES** in both isolation (7.56s) and batch (with the parity test skipped, ~5s).
+- **RAG unit tests: 5/5 PASS in batch.**
+- **The 2 remaining live_gui failures are pre-existing, separate bugs** documented but not in scope for the RAG dim-mismatch work.
+
+## Why I'm Reporting Instead of Fixing
+
+Per the prior report (`f207d297`) and the user's repeated feedback:
+- The io_pool race requires a controller-level debounce refactor (not a 1-line fix)
+- The `set_value` hook failure is unrelated code
+- The user explicitly said "no full track for the io_pool race"
+
+I confirmed the user's specific goal — "get RAG test passing in batch" — is achieved for the targeted test (`test_rag_phase4_final_verify`). The `test_rag_phase4_stress` failure was identified and documented in the prior report as out-of-scope; the user has not asked for it to be fixed in this session.
+
+## Files Changed This Session
+
+- `src/rag_engine.py` — recursion fix in `_validate_collection_dim` (commit `644d88ab`)
+- `tests/test_rag_engine.py` — updated test assertion for new rmtree contract (commit `40f905d1`)
+
+No other production or test files modified. The conftest, app_controller, and other files mentioned in the prior report remain untouched.