refactor(rag_engine): obliterate legacy _chunk_code wrapper (Phase 5)
Phase 5 (1 of 9 cruft sites obliterated): OBLITERATED: RAGEngine._chunk_code wrapper. It delegated to _chunk_code_result and provided a fallback to _chunk_text on AST failure. Migration: index_file() now calls _chunk_code_result directly with .ok check + chunk-size threshold check + fallback to _chunk_text inline. The structured ErrorInfo is propagated if needed (no caller currently consumes it). Sub-track 5 tests updated: - tests/tier2/phase13_invariant_test.py: _chunk_code moved to obliterated list - tests/tier2/phase13_site2_test.py: _legacy_no_broad_except -> _legacy_obliterated - tests/test_cruft_removal.py: 2 new tests (wrapper-obliterated invariant + caller-uses-result invariant) PITFALL encountered: the edit_file tool removed a leading space on the next class method's 'def' line, causing an IndentationError. Fixed by binary-write replacement preserving CRLF + leading-space styleguide convention (project uses 1-space indentation; class body methods start at column 1). Test result: 124/124 pass. Audit gate: src/rag_engine.py --strict exits 0 (no new violations). Wrapper count: 3 -> 2 (Phase 6 remaining: gui_2 2).
This commit is contained in:
+5
-11
@@ -247,16 +247,6 @@ class RAGEngine:
|
||||
)
|
||||
|
||||
|
||||
def _chunk_code(self, content: str, file_path: str) -> List[str]:
|
||||
"""AST-aware chunking for Python code."""
|
||||
ast_result = self._chunk_code_result(content, file_path)
|
||||
if not ast_result.ok:
|
||||
return self._chunk_text(content)
|
||||
chunks = ast_result.data
|
||||
if not chunks or len(content) < self.config.chunk_size:
|
||||
return self._chunk_text(content)
|
||||
return chunks
|
||||
|
||||
def _get_file_mtime_result(self, full_path: str) -> Result[float]:
|
||||
"""Get file modification time. Returns Result[float]."""
|
||||
try:
|
||||
@@ -331,7 +321,11 @@ class RAGEngine:
|
||||
self.collection.delete(where={"path": file_path})
|
||||
|
||||
if file_path.lower().endswith(".py"):
|
||||
chunks = self._chunk_code(content, file_path)
|
||||
chunk_result = self._chunk_code_result(content, file_path)
|
||||
if not chunk_result.ok or not chunk_result.data or len(content) < self.config.chunk_size:
|
||||
chunks = self._chunk_text(content)
|
||||
else:
|
||||
chunks = chunk_result.data
|
||||
else:
|
||||
chunks = self._chunk_text(content)
|
||||
|
||||
|
||||
@@ -120,4 +120,30 @@ def test_phase4_list_minimax_models_wrapper_obliterated():
|
||||
from src import ai_client
|
||||
assert not hasattr(ai_client, "_list_minimax_models"), (
|
||||
"_list_minimax_models legacy wrapper must be OBLITERATED."
|
||||
)
|
||||
|
||||
|
||||
# ============ Phase 5 (rag_engine wrappers) ============
|
||||
|
||||
def test_phase5_chunk_code_wrapper_obliterated():
|
||||
"""Phase 5 invariant: the legacy RAGEngine._chunk_code wrapper is DELETED."""
|
||||
from src.rag_engine import RAGEngine
|
||||
assert not hasattr(RAGEngine, "_chunk_code"), (
|
||||
"RAGEngine._chunk_code legacy wrapper must be OBLITERATED."
|
||||
)
|
||||
|
||||
|
||||
def test_phase5_chunk_code_caller_uses_result():
|
||||
"""The caller in index_file uses _chunk_code_result(...).ok directly."""
|
||||
from src.rag_engine import RAGEngine
|
||||
import inspect
|
||||
src_text = inspect.getsource(RAGEngine.index_file)
|
||||
assert "_chunk_code_result" in src_text, "caller must use _chunk_code_result"
|
||||
# Check no bare _chunk_code( call (without _result suffix)
|
||||
import re
|
||||
bare_calls = re.findall(r"\b_chunk_code\(", src_text)
|
||||
bare_calls = [c for c in bare_calls if c == "_chunk_code("]
|
||||
assert len(bare_calls) == 0, (
|
||||
f"caller should not call _chunk_code (legacy wrapper); "
|
||||
f"found {len(bare_calls)} bare calls"
|
||||
)
|
||||
@@ -58,12 +58,19 @@ def test_phase13_all_helpers_exist():
|
||||
|
||||
|
||||
def test_phase13_legacy_functions_preserved():
|
||||
"""All legacy functions must still exist + be callable."""
|
||||
"""All legacy functions must still exist + be callable (some OBLITERATED by cruft-removal)."""
|
||||
import src.rag_engine
|
||||
# Class methods
|
||||
for name in ("_chunk_code", "_search_mcp", "search", "delete_documents",
|
||||
"get_all_indexed_paths", "delete_documents_by_path", "index_file"):
|
||||
# Class methods (some OBLITERATED)
|
||||
preserved = ("_search_mcp", "search", "delete_documents",
|
||||
"get_all_indexed_paths", "delete_documents_by_path", "index_file")
|
||||
obliterated = ("_chunk_code",)
|
||||
for name in preserved:
|
||||
assert hasattr(src.rag_engine.RAGEngine, name), f"{name} method missing"
|
||||
for name in obliterated:
|
||||
assert not hasattr(src.rag_engine.RAGEngine, name), (
|
||||
f"{name} wrapper must be OBLITERATED (cruft-removal Phase 5); "
|
||||
f"callers must use {name}_result directly"
|
||||
)
|
||||
# Module-level
|
||||
for name in ("_get_sentence_transformers", "_get_google_genai", "_get_chromadb"):
|
||||
assert hasattr(src.rag_engine, name), f"{name} module-level function missing"
|
||||
@@ -24,10 +24,10 @@ def test_phase13_site2_chunk_code_result_exists():
|
||||
"_chunk_code_result helper missing"
|
||||
|
||||
|
||||
def test_phase13_site2_chunk_code_legacy_no_broad_except():
|
||||
"""Legacy _chunk_code must NOT have bare 'except Exception'."""
|
||||
import inspect
|
||||
def test_phase13_site2_chunk_code_legacy_obliterated():
|
||||
"""_chunk_code wrapper OBLITERATED by cruft-removal Phase 5."""
|
||||
import src.rag_engine
|
||||
src_text = inspect.getsource(src.rag_engine.RAGEngine._chunk_code)
|
||||
assert "except Exception:" not in src_text, \
|
||||
"_chunk_code legacy must not have bare 'except Exception'"
|
||||
assert not hasattr(src.rag_engine.RAGEngine, "_chunk_code"), (
|
||||
"_chunk_code wrapper must be DELETED; "
|
||||
"callers must use _chunk_code_result(...).ok directly."
|
||||
)
|
||||
Reference in New Issue
Block a user