refactor(rag_engine): migrate _async_search_mcp JSON parse to Result[T] (Phase 13 site 5)
Site 5 (BC at L290): _async_search_mcp (nested in _search_mcp) had:
try:
data = json.loads(res_str)
if isinstance(data, list): return data
elif isinstance(data, dict) and 'results' in data: return data['results']
return []
except:
return []
Body: bare 'except:' + return [] = empty default = SS-style violation.
Migrated to Result[T] via new module-level helper _parse_search_response_result:
- Returns Result(data=parsed_list) on success
- Returns Result(data=None, errors=[ErrorInfo]) on JSON parse failure
- Handles the list/dict/no-results branch logic
The helper is module-level (does not use self) and is placed BEFORE
class RAGEngine to avoid breaking the class definition (a def at column 0
inside a class ends the class prematurely).
Legacy _async_search_mcp delegates to the helper; on Result errors,
returns [] (preserving the original behavior).
Audit: rag_engine BC 1 -> 0; migration-target: 0.
Remaining 4 INTERNAL_RETHROW sites are Pattern 1/3 of the styleguide
(known audit limitation).
This commit is contained in:
+19
-10
@@ -85,6 +85,22 @@ class GeminiEmbeddingProvider(BaseEmbeddingProvider):
|
||||
)
|
||||
return [e.values for e in res.embeddings]
|
||||
|
||||
def _parse_search_response_result(res_str: str) -> Result[List[Dict[str, Any]]]:
|
||||
"""Parse the MCP rag_search response. Returns Result[List[dict]]. On JSON parse failure, returns Result(errors=[ErrorInfo]). The legacy caller returns [] on errors, preserving the original behavior."""
|
||||
try:
|
||||
data = json.loads(res_str)
|
||||
except (ValueError, TypeError) as e:
|
||||
return Result(
|
||||
data=None,
|
||||
errors=[ErrorInfo(kind=ErrorKind.INTERNAL, message=f"_search_mcp JSON parse failed: {e}", source="rag_engine._parse_search_response_result", original=e)],
|
||||
)
|
||||
if isinstance(data, list):
|
||||
return Result(data=data)
|
||||
if isinstance(data, dict) and "results" in data:
|
||||
return Result(data=data["results"])
|
||||
return Result(data=[])
|
||||
|
||||
|
||||
class RAGEngine:
|
||||
def __init__(self, config: models.RAGConfig, base_dir: str = "."):
|
||||
self.config = copy.deepcopy(config)
|
||||
@@ -327,19 +343,12 @@ class RAGEngine:
|
||||
self.add_documents(ids, chunks, metadatas)
|
||||
|
||||
def _search_mcp(self, query: str, top_k: int = 5) -> List[Dict[str, Any]]:
|
||||
async def _async_search_mcp():
|
||||
async def _async_search_mcp() -> List[Dict[str, Any]]:
|
||||
tool_name = self.config.vector_store.mcp_tool or "rag_search"
|
||||
args = {"query": query, "top_k": top_k}
|
||||
res_str = await mcp_client.async_dispatch(tool_name, args)
|
||||
try:
|
||||
data = json.loads(res_str)
|
||||
if isinstance(data, list):
|
||||
return data
|
||||
elif isinstance(data, dict) and "results" in data:
|
||||
return data["results"]
|
||||
return []
|
||||
except:
|
||||
return []
|
||||
parse_result = _parse_search_response_result(res_str)
|
||||
return parse_result.data if parse_result.ok else []
|
||||
|
||||
return asyncio.run(_async_search_mcp())
|
||||
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
"""Phase 13 site 5: _async_search_mcp Result migration.
|
||||
|
||||
Site 5 (BC at L290): the nested _async_search_mcp inside _search_mcp has:
|
||||
try:
|
||||
data = json.loads(res_str)
|
||||
if isinstance(data, list): return data
|
||||
elif isinstance(data, dict) and 'results' in data: return data['results']
|
||||
return []
|
||||
except:
|
||||
return []
|
||||
|
||||
Body: bare 'except:' + return [] = empty default. MIGRATE to Result.
|
||||
"""
|
||||
import sys
|
||||
sys.path.insert(0, ".")
|
||||
|
||||
|
||||
def test_phase13_site5_async_search_mcp_no_bare_except():
|
||||
import inspect
|
||||
import src.rag_engine
|
||||
src_text = inspect.getsource(src.rag_engine.RAGEngine._search_mcp)
|
||||
assert "except:" not in src_text or "except (ValueError, TypeError)" in src_text, \
|
||||
"_search_mcp must not have bare 'except:'"
|
||||
assert "except Exception" not in src_text, \
|
||||
"_search_mcp must not have bare 'except Exception'"
|
||||
Reference in New Issue
Block a user