fix(rag): convert RAGChunk to dict in _rag_search_result to match type contract
The RAG engine's search() returns List[RAGChunk] (dataclass instances),
but _rag_search_result's return type is Result[list[Metadata]] (a list
of dicts). The previous code returned the RAGChunks as-is, then the
caller in _handle_request_event did chunk["metadata"] (dict access
on a dataclass) which raised TypeError. The exception was silently
swallowed by the submit_io worker, leaving ai_status stuck at
sending... for the full 50-second test poll before failing.
Two surgical changes:
1. _rag_search_result: convert RAGChunk to dict via to_dict() (with a
hasattr guard for tests that return dicts directly). Matches the
function's documented return type.
2. _handle_request_event: use isinstance guards + dict.get() on the
chunk fields. Defensive against the type mismatch and matches the
dict contract.
The test fix (unique collection name + workspace-targeted cleanup)
is the test-side complement that prevents the dim-mismatch path from
being hit in batched runs.
Verified: 4 consecutive PASS runs of test_rag_phase4_final_verify in
isolation (7-8s each). 25/26 RAG tests pass; the one remaining
failure (test_rag_collection_dim_mismatch_recreates_collection) is a
pre-existing regression from commit 24e93a75 which changed the dim
check from delete_collection to shutil.rmtree without updating the
test mock setup. Out of scope for this fix.
This commit is contained in:
+11
-8
@@ -3501,14 +3501,17 @@ class AppController:
|
|||||||
|
|
||||||
def _rag_search_result(self, user_msg: str) -> "Result[list[Metadata]]":
|
def _rag_search_result(self, user_msg: str) -> "Result[list[Metadata]]":
|
||||||
"""Per-event handler (Phase 6 Group 6.6): RAG search via the engine.
|
"""Per-event handler (Phase 6 Group 6.6): RAG search via the engine.
|
||||||
Returns Result[List[RAGChunk]]. On failure: any engine/SDK exception
|
Returns Result[List[Metadata]] (list of dicts per the function's
|
||||||
-> ErrorInfo(original=e). Caller (`_handle_request_event`) appends
|
type annotation; the engine's search() returns List[RAGChunk]
|
||||||
|
dataclasses which are converted via to_dict() here so the caller
|
||||||
|
in `_handle_request_event` can use dict-style access). On failure:
|
||||||
|
any engine/SDK exception -> ErrorInfo(original=e). Caller appends
|
||||||
to `self._last_request_errors` for sub-track 4 GUI display."""
|
to `self._last_request_errors` for sub-track 4 GUI display."""
|
||||||
if not (self.rag_engine and self.rag_config and self.rag_config.enabled):
|
if not (self.rag_engine and self.rag_config and self.rag_config.enabled):
|
||||||
return Result(data=[])
|
return Result(data=[])
|
||||||
try:
|
try:
|
||||||
chunks = self.rag_engine.search(user_msg)
|
chunks = self.rag_engine.search(user_msg)
|
||||||
return Result(data=list(chunks) if chunks else [])
|
return Result(data=[c.to_dict() if hasattr(c, "to_dict") and not isinstance(c, dict) else (c if isinstance(c, dict) else Metadata()) for c in chunks])
|
||||||
except (OSError, IOError, ValueError, TypeError, KeyError, AttributeError, RuntimeError) as e:
|
except (OSError, IOError, ValueError, TypeError, KeyError, AttributeError, RuntimeError) as e:
|
||||||
return Result(data=[], errors=[ErrorInfo(
|
return Result(data=[], errors=[ErrorInfo(
|
||||||
kind=ErrorKind.NETWORK,
|
kind=ErrorKind.NETWORK,
|
||||||
@@ -4171,18 +4174,18 @@ class AppController:
|
|||||||
[C: tests/test_live_gui_integration_v2.py:test_user_request_error_handling, tests/test_live_gui_integration_v2.py:test_user_request_integration_flow, tests/test_rag_integration.py:test_rag_integration]
|
[C: tests/test_live_gui_integration_v2.py:test_user_request_error_handling, tests/test_live_gui_integration_v2.py:test_user_request_integration_flow, tests/test_rag_integration.py:test_rag_integration]
|
||||||
"""
|
"""
|
||||||
self.ai_status = 'sending...'
|
self.ai_status = 'sending...'
|
||||||
|
|
||||||
user_msg = event.prompt
|
user_msg = event.prompt
|
||||||
|
|
||||||
# 1. RAG Retrieval (Enrich prompt before logging to history)
|
# 1. RAG Retrieval (Enrich prompt before logging to history)
|
||||||
if self.rag_engine and self.rag_config and self.rag_config.enabled:
|
if self.rag_engine and self.rag_config and self.rag_config.enabled:
|
||||||
rag_result = self._rag_search_result(user_msg)
|
rag_result = self._rag_search_result(user_msg)
|
||||||
if rag_result.ok and rag_result.data:
|
if rag_result.ok and rag_result.data:
|
||||||
context_block = "## Retrieved Context\n\n"
|
context_block = "## Retrieved Context\n\n"
|
||||||
for i, chunk in enumerate(rag_result.data):
|
for i, chunk in enumerate(rag_result.data):
|
||||||
chunk_meta = chunk["metadata"] if "metadata" in chunk else {}
|
chunk_meta = chunk.get("metadata", {}) if isinstance(chunk, dict) else {}
|
||||||
path = chunk_meta["path"] if "path" in chunk_meta else "unknown"
|
path = chunk_meta.get("path", "unknown") if isinstance(chunk_meta, dict) else "unknown"
|
||||||
doc = chunk["document"] if "document" in chunk else ""
|
doc = chunk.get("document", "") if isinstance(chunk, dict) else ""
|
||||||
context_block += f"### Chunk {i+1} (Source: {path})\n{doc}\n\n"
|
context_block += f"### Chunk {i+1} (Source: {path})\n{doc}\n\n"
|
||||||
user_msg = context_block + user_msg
|
user_msg = context_block + user_msg
|
||||||
elif not rag_result.ok:
|
elif not rag_result.ok:
|
||||||
|
|||||||
@@ -17,23 +17,28 @@ def test_phase4_final_verify(live_gui, live_gui_workspace):
|
|||||||
client = api_hook_client.ApiHookClient()
|
client = api_hook_client.ApiHookClient()
|
||||||
assert client.wait_for_server(timeout=15), "Hook server did not start"
|
assert client.wait_for_server(timeout=15), "Hook server did not start"
|
||||||
|
|
||||||
# Clean the chroma cache BEFORE the test starts. In batched live_gui
|
# Use a unique collection name per test invocation. The RAG engine
|
||||||
# context, the live_gui subprocess is shared across many tests, and
|
# stores its chromadb collection at
|
||||||
# prior tests leave chroma state at the controller's project root
|
# <base_dir>/.slop_cache/chroma_<collection_name>. The live_gui
|
||||||
# (e.g. C:\projects\manual_slop\tests\artifacts\.slop_cache\chroma_test_*).
|
# subprocess holds a Windows file lock on the chroma sqlite file
|
||||||
# The dim-mismatch rmtree in rag_engine._validate_collection_dim
|
# from the prior test (WinError 32), so the rmtree in
|
||||||
# fails on Windows with WinError 32 (file in use), leaving a stale
|
# _validate_collection_dim is a no-op on locked files, leaving the
|
||||||
# locked collection that PersistentClient can't open. Wipe the
|
# collection with a stale dim (e.g. 3072 from a prior Gemini
|
||||||
# relevant cache dirs proactively so the test starts clean.
|
# embedding pass) that breaks subsequent searches (hangs on
|
||||||
_workspace_root = str(live_gui_workspace.parent if live_gui_workspace else Path.cwd())
|
# dim mismatch). A unique name avoids the collision entirely so the
|
||||||
stale_path = Path(_workspace_root) / ".slop_cache"
|
# dim check is a no-op and the test gets a fresh collection.
|
||||||
if stale_path.exists():
|
_collection_name = f"test_final_verify_{int(time.time() * 1000)}"
|
||||||
for col_dir in stale_path.iterdir():
|
|
||||||
|
# Best-effort cleanup of the workspace's .slop_cache (where the
|
||||||
|
# chroma collection actually lives). ignore_errors=True handles
|
||||||
|
# WinError 32 from the live_gui subprocess's file lock. This is
|
||||||
|
# belt-and-suspenders; the unique collection name above is the
|
||||||
|
# primary defense.
|
||||||
|
_slop_cache = Path(live_gui_workspace) / ".slop_cache"
|
||||||
|
if _slop_cache.exists():
|
||||||
|
for col_dir in _slop_cache.iterdir():
|
||||||
if col_dir.is_dir() and col_dir.name.startswith("chroma_"):
|
if col_dir.is_dir() and col_dir.name.startswith("chroma_"):
|
||||||
try:
|
shutil.rmtree(col_dir, ignore_errors=True)
|
||||||
shutil.rmtree(col_dir)
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
|
|
||||||
# 1. Setup mock project data
|
# 1. Setup mock project data
|
||||||
workspace_dir = live_gui_workspace
|
workspace_dir = live_gui_workspace
|
||||||
@@ -45,7 +50,7 @@ def test_phase4_final_verify(live_gui, live_gui_workspace):
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
# 2. Configure project through Hook API
|
# 2. Configure project through Hook API
|
||||||
client.set_value('rag_collection_name', 'test_final_verify')
|
client.set_value('rag_collection_name', _collection_name)
|
||||||
client.set_value('files', ['final_test_1.txt', 'final_test_2.py'])
|
client.set_value('files', ['final_test_1.txt', 'final_test_2.py'])
|
||||||
client.set_value('rag_enabled', True)
|
client.set_value('rag_enabled', True)
|
||||||
client.set_value('rag_source', 'chroma')
|
client.set_value('rag_source', 'chroma')
|
||||||
|
|||||||
Reference in New Issue
Block a user