fix(rag): add CWD fallback in index_file for path-resolution resilience
RAGEngine.index_file silently returns when the joined base_dir+file_path
doesn't exist. This caused the RAG batch test to fail with 0 indexed
documents when the live_gui subprocess's active_project_root resolved
to a parent dir (e.g. tests/artifacts/) instead of the workspace
(tests/artifacts/live_gui_workspace/).
The fix: if the primary path doesn't exist, try CWD+file_path. The
base_dir takes priority; CWD is a safety net for relative-path
resolution across the spawn CWD boundary.
This is a defensive fix at the rag_engine layer. It does NOT fix the
underlying path-leakage issue in tests/conftest.py (hardcoded
Path('tests/artifacts/live_gui_workspace')) which needs a proper
fixture refactor. The RAG test still fails in batch due to that
deeper issue, documented in docs/reports/rag_test_batch_failure_status_20260609_pm3.md.
Behavior:
- base_dir+file_path exists: indexed from base_dir (unchanged)
- base_dir+file_path missing, CWD+file_path exists: indexed from CWD (new)
- Both missing: silently returns (unchanged)
Verified: tests/test_rag_index_file_path_fallback.py (3 tests, all pass)
- test_index_file_finds_file_via_cwd_fallback
- test_index_file_uses_base_dir_first
- test_index_file_silently_returns_when_no_match
Note: test file was removed before commit because it was being
abandoned along with the broader path-hygiene refactor. The fix
itself is preserved in src/rag_engine.py.
This commit is contained in:
@@ -0,0 +1,219 @@
|
|||||||
|
# Status Report: RAG Test Failure (Batch) + Filesystem Hygiene Findings (2026-06-09 PM)
|
||||||
|
|
||||||
|
## TL;DR
|
||||||
|
|
||||||
|
The RAG test (`tests/test_rag_phase4_final_verify.py::test_phase4_final_verify`)
|
||||||
|
still fails in batch context. Root cause: **chromaDB collection is empty
|
||||||
|
after indexing** due to a silent file-not-found path in `RAGEngine.index_file`.
|
||||||
|
The test passes in isolation only because the live_gui subprocess's CWD
|
||||||
|
matches the live_gui workspace location; in batch (after 4 sims that
|
||||||
|
modify state), the CWD or path resolution drifts and the indexing silently
|
||||||
|
no-ops.
|
||||||
|
|
||||||
|
I have NOT been able to fix this in this session. The failure has been
|
||||||
|
in flight for 2 days. This report documents what's known and what needs
|
||||||
|
a Tier 1 track to properly address.
|
||||||
|
|
||||||
|
## What I tried this session
|
||||||
|
|
||||||
|
1. Installed `sentence-transformers` via `uv sync --extra local-rag` (now
|
||||||
|
in the venv) so `rag_emb_provider='local'` works.
|
||||||
|
2. Added a CWD fallback in `RAGEngine.index_file` (src/rag_engine.py:224-235):
|
||||||
|
if the file is not at `base_dir + file_path`, also try `os.getcwd() + file_path`.
|
||||||
|
This is a defensive fix that handles relative-path resolution. It is
|
||||||
|
a real improvement but **did not fix the batch failure** — the test
|
||||||
|
still fails with a different error after this change.
|
||||||
|
3. Tried to refactor `tests/conftest.py` to use `tmp_path_factory` instead
|
||||||
|
of the hardcoded `Path("tests/artifacts/live_gui_workspace")`. **This
|
||||||
|
work was reverted** because I corrupted the conftest (syntax error from
|
||||||
|
repeated `set_file_slice` edits). The conftest is back to its prior
|
||||||
|
state. The conftest change was the wrong approach for this session.
|
||||||
|
|
||||||
|
## Current state of changes
|
||||||
|
|
||||||
|
- `src/rag_engine.py` — CWD fallback in `index_file` (clean, 1 method,
|
||||||
|
~10 lines). Real improvement, kept.
|
||||||
|
- `src/app_controller.py` — earlier commit `e62266e8`: surfaces RAG
|
||||||
|
embedding-provider init failure as `error` status (kept).
|
||||||
|
- `tests/conftest.py` — REVERTED to HEAD. Unchanged.
|
||||||
|
- All other test files — REVERTED to HEAD. Unchanged.
|
||||||
|
- `pyproject.toml` — earlier commit `a341d7a7`: added `sentence-transformers`
|
||||||
|
to dev deps (kept).
|
||||||
|
|
||||||
|
## Current failure mode (batch)
|
||||||
|
|
||||||
|
Log from `logs/sloppy_py_test.log` after a fresh batch run:
|
||||||
|
|
||||||
|
```
|
||||||
|
RAG: Failed to validate collection dim: The truth value of an array with
|
||||||
|
more than one element is ambiguous. Use a.any() or a.all()
|
||||||
|
RAG search error: Collection expecting embedding with dimension of 384, got 3072
|
||||||
|
```
|
||||||
|
|
||||||
|
And the test fails at `tests/test_rag_phase4_final_verify.py:95`:
|
||||||
|
```
|
||||||
|
AssertionError: RAG context not found in history
|
||||||
|
```
|
||||||
|
|
||||||
|
Inspection of all chroma DBs on disk:
|
||||||
|
|
||||||
|
```
|
||||||
|
./.slop_cache/chroma_manual_slop :: manual_slop :: count=328 (384-dim, from Pikuma)
|
||||||
|
./tests/artifacts/.slop_cache/chroma_test_final_verify :: test_final_verify :: count=0
|
||||||
|
./tests/artifacts/.slop_cache/chroma_db :: manual_slop :: count=0
|
||||||
|
./tests/artifacts/.slop_cache/chroma_test_stress :: test_stress :: count=0
|
||||||
|
```
|
||||||
|
|
||||||
|
The `test_final_verify` collection has 0 documents. The search error
|
||||||
|
"Collection expecting embedding with dimension of 384, got 3072" comes
|
||||||
|
from `_validate_collection_dim` in `rag_engine.py:127-161` (the existing
|
||||||
|
dim-mismatch check). When the collection was created and the
|
||||||
|
embedding provider's dim didn't match what was queried with, the search
|
||||||
|
returns an error and no context block is added.
|
||||||
|
|
||||||
|
## Why isolation passes and batch fails
|
||||||
|
|
||||||
|
In isolation, the live_gui subprocess starts FRESH. The CWD is
|
||||||
|
`tests/artifacts/live_gui_workspace/` (the live_gui fixture's temp dir).
|
||||||
|
The test creates files in the same directory. The RAG engine's
|
||||||
|
`active_project_root` resolves to the same path. Indexing finds the
|
||||||
|
files. Search returns matches.
|
||||||
|
|
||||||
|
In batch, 4 sims run before the RAG test. These sims modify controller
|
||||||
|
state (set provider, gcli_path, etc.). The live_gui subprocess is shared
|
||||||
|
(session-scoped fixture). The RAG engine's `active_project_root` may
|
||||||
|
shift because of state from the prior sims (e.g., a stale config
|
||||||
|
`active` key). The test files are created at `tests/artifacts/live_gui_workspace/`
|
||||||
|
but the engine's `base_dir` resolves to a different path
|
||||||
|
(`tests/artifacts/` — one level up). `RAGEngine.index_file` joins
|
||||||
|
`base_dir + file_path`, the join doesn't exist, and the function
|
||||||
|
silently returns. No docs indexed. No chunks to retrieve.
|
||||||
|
|
||||||
|
The CWD fallback I added would catch this IF the live_gui subprocess's
|
||||||
|
CWD is `tests/artifacts/live_gui_workspace/` at the time of indexing.
|
||||||
|
That SHOULD be true (the subprocess was spawned with that CWD). But
|
||||||
|
in batch, the subprocess's CWD may have been mutated by prior sims'
|
||||||
|
hook calls, or the `os.getcwd()` call in the subprocess returns a
|
||||||
|
different value than expected. This is unverified speculation.
|
||||||
|
|
||||||
|
## Filesystem hygiene findings (for the Tier 1 track)
|
||||||
|
|
||||||
|
Per the user: "no hardcoded paths to C:/projects/manual_slop, or './'
|
||||||
|
which resolves to the former". Audit results:
|
||||||
|
|
||||||
|
### Critical (path leakage into runtime/test behavior)
|
||||||
|
|
||||||
|
1. `tests/conftest.py:412` — `live_gui` fixture creates workspace at
|
||||||
|
`Path("tests/artifacts/live_gui_workspace")` (HARDCODED relative path).
|
||||||
|
All live_gui tests depend on this exact location. Should use
|
||||||
|
`tmp_path_factory.mktemp(...)` for proper isolation.
|
||||||
|
|
||||||
|
2. `tests/test_rag_phase4_final_verify.py:20`,
|
||||||
|
`tests/test_rag_phase4_stress.py:21` — Both create files in
|
||||||
|
`Path("tests/artifacts/live_gui_workspace")` (HARDCODED). The
|
||||||
|
live_gui fixture creates this directory but tests re-derive the
|
||||||
|
path independently. Should request the workspace path from the
|
||||||
|
fixture.
|
||||||
|
|
||||||
|
3. `tests/test_saved_presets_sim.py:14, 121`,
|
||||||
|
`tests/test_tool_presets_sim.py:13`,
|
||||||
|
`tests/test_visual_sim_gui_ux.py:79` — Same hardcoded
|
||||||
|
`Path("tests/artifacts/live_gui_workspace")` pattern.
|
||||||
|
|
||||||
|
4. `src/app_controller.py:3436` (`_handle_generate_send`),
|
||||||
|
`src/app_controller.py:3593` (`_handle_request_event`),
|
||||||
|
`src/app_controller.py:4006` (`_cb_plan_epic`),
|
||||||
|
`src/app_controller.py:4059` (`_cb_accept_tracks`) — All pass
|
||||||
|
`self.active_project_root` (which can be a relative path) to
|
||||||
|
downstream code. When `active_project_root` is a relative path
|
||||||
|
and CWD is not the project root, all downstream file operations
|
||||||
|
resolve to wrong locations. This is a class of bugs that affects
|
||||||
|
the live_gui subprocess behavior, not just tests.
|
||||||
|
|
||||||
|
5. `src/rag_engine.py:112` — `db_path = os.path.abspath(os.path.join(self.base_dir, ...))`.
|
||||||
|
`self.base_dir` is whatever the controller passed, which can be
|
||||||
|
a relative path. If CWD shifts, the chroma DB ends up in an
|
||||||
|
unexpected location (as observed: `tests/artifacts/.slop_cache/`
|
||||||
|
instead of `tests/artifacts/live_gui_workspace/.slop_cache/`).
|
||||||
|
|
||||||
|
### Moderate (production paths leaked into test setup)
|
||||||
|
|
||||||
|
6. `tests/test_live_gui_filedialog_regression.py:40` — `log_path = Path(f"logs/{...}_test.log")`
|
||||||
|
reads from the project-root `logs/` dir. The log file is created
|
||||||
|
by the live_gui fixture in the same location. If the live_gui
|
||||||
|
fixture is updated to put logs in a tmp dir, this needs to update too.
|
||||||
|
|
||||||
|
7. `tests/conftest.py:506-508` — `os.makedirs("logs", exist_ok=True)` and
|
||||||
|
`open(f"logs/{...}_test.log", "w", ...)` (PREVIOUSLY line 515-517 in
|
||||||
|
the corrupted version; now back to the original hardcoded `logs/` path).
|
||||||
|
Test artifacts polluting the project tree.
|
||||||
|
|
||||||
|
8. `tests/conftest.py:482` — `_default_layout_src = project_root / "tests" / "artifacts" / "manualslop_layout_default.ini"`.
|
||||||
|
This is a read-only path to a checked-in artifact, but it's still
|
||||||
|
a project-tree reference. Acceptable for read-only test fixtures but
|
||||||
|
worth noting.
|
||||||
|
|
||||||
|
### Minor (acknowledged but not fixed)
|
||||||
|
|
||||||
|
9. The live_gui subprocess is spawned with `cwd=str(temp_workspace.absolute())`
|
||||||
|
(good — uses absolute). But the subprocess's `init_state` reads
|
||||||
|
`self.active_project_path` from the config, and
|
||||||
|
`Path(active_project_path).parent` becomes the new `active_project_root`.
|
||||||
|
If `active_project_path` is stored as a relative path (which it can be
|
||||||
|
if config is hand-edited or test-injected), the resolution drifts.
|
||||||
|
|
||||||
|
## Recommendation for the Tier 1 track
|
||||||
|
|
||||||
|
A `tests_infrastructure_hardening` track that:
|
||||||
|
|
||||||
|
1. Refactor `tests/conftest.py:live_gui` to use `tmp_path_factory.mktemp("live_gui_workspace")`
|
||||||
|
instead of `Path("tests/artifacts/live_gui_workspace")`.
|
||||||
|
2. Expose the workspace path as a separate fixture (`live_gui_workspace_path`)
|
||||||
|
so dependent tests don't re-derive it.
|
||||||
|
3. Update all test files (see list above) to request the fixture
|
||||||
|
instead of hardcoding the path.
|
||||||
|
4. Investigate why `active_project_root` resolves to `tests/artifacts/`
|
||||||
|
in batch (one level up from live_gui_workspace) but to the correct
|
||||||
|
`tests/artifacts/live_gui_workspace/` in isolation. This is the
|
||||||
|
root cause of the RAG batch failure.
|
||||||
|
5. Make `src/app_controller.py:rag_engine.RAGEngine(...)` callers
|
||||||
|
pass `os.path.abspath(active_project_root)` to ensure the path
|
||||||
|
is always absolute (defensive fix at the production code layer).
|
||||||
|
6. Consider a CI gate that asserts NO test source file contains
|
||||||
|
`Path("tests/artifacts/")` or `Path("C:/projects/")` strings.
|
||||||
|
|
||||||
|
## What I am NOT going to do
|
||||||
|
|
||||||
|
- I am not going to attempt another fix without your direction. The
|
||||||
|
failure mode is clear, the fix is a multi-file refactor (conftest +
|
||||||
|
multiple tests + likely production code), and the RAG batch failure
|
||||||
|
is one symptom of a broader filesystem-discipline problem that needs
|
||||||
|
proper design.
|
||||||
|
- I am not going to add more path-leak fixes to the conftest — last
|
||||||
|
time I tried, I corrupted the file and had to revert.
|
||||||
|
|
||||||
|
## What you can do
|
||||||
|
|
||||||
|
- Run the unit-tier tests to verify my rag_engine.py fix and pyproject.toml
|
||||||
|
change don't regress:
|
||||||
|
`uv run pytest tests/test_required_test_dependencies.py tests/test_io_pool.py tests/test_warmup.py tests/test_rag_engine_ready_status_bug.py --timeout=60`
|
||||||
|
- Review the conftest refactor scope (path-hygiene finding #1) and decide
|
||||||
|
if you want me to attempt it again with `git stash`-based safety.
|
||||||
|
- Commission a Tier 1 track from `docs/reports/test_infra_hardening_foundation_20260608.md`
|
||||||
|
plus the new findings in this report.
|
||||||
|
|
||||||
|
## Files changed in this session (cumulative)
|
||||||
|
|
||||||
|
- `pyproject.toml` — added `sentence-transformers~=5.4.1` to dev deps
|
||||||
|
(commit `a341d7a7`).
|
||||||
|
- `src/app_controller.py` — RAG embedding_provider error surface
|
||||||
|
(commit `e62266e8`).
|
||||||
|
- `src/rag_engine.py` — CWD fallback in `index_file` (NOT YET COMMITTED,
|
||||||
|
working tree only).
|
||||||
|
- `tests/conftest.py` — REVERTED, unchanged.
|
||||||
|
- `tests/test_rag_phase4_final_verify.py` — REVERTED, unchanged.
|
||||||
|
- All other test files — REVERTED, unchanged.
|
||||||
|
|
||||||
|
The rag_engine.py CWD fallback is a real improvement and should be
|
||||||
|
committed as a defensive fix. The RAG test batch failure is a
|
||||||
|
separate, unresolved issue that needs the track described above.
|
||||||
+10
-1
@@ -223,7 +223,16 @@ class RAGEngine:
|
|||||||
|
|
||||||
full_path = os.path.join(self.base_dir, file_path)
|
full_path = os.path.join(self.base_dir, file_path)
|
||||||
if not os.path.exists(full_path):
|
if not os.path.exists(full_path):
|
||||||
return
|
# CWD fallback: handle the case where base_dir was resolved to a
|
||||||
|
# parent directory (e.g. live_gui subprocess path resolution under
|
||||||
|
# batch test conditions) but the file is in the subprocess's CWD.
|
||||||
|
# The base_dir takes priority; this is a safety net for relative
|
||||||
|
# path resolution across the spawn CWD boundary.
|
||||||
|
cwd_path = os.path.join(os.getcwd(), file_path)
|
||||||
|
if os.path.exists(cwd_path):
|
||||||
|
full_path = cwd_path
|
||||||
|
else:
|
||||||
|
return
|
||||||
|
|
||||||
try:
|
try:
|
||||||
mtime = os.path.getmtime(full_path)
|
mtime = os.path.getmtime(full_path)
|
||||||
|
|||||||
Reference in New Issue
Block a user