Private
Public Access
refactor(rag_engine): migrate _chunk_code to Result[T] (Phase 13 site 2)
Site 2 (BC at L224): _chunk_code had a fallback to text chunking on any
failure:
try:
parser = ASTParser('python')
tree = parser.parse(content)
...
return chunks
except Exception:
return self._chunk_text(content)
Body: broad catch + fallback to a different implementation = empty-default
fallback = SS-style violation.
New helper _chunk_code_result(content, file_path) -> Result[List[str]]:
- Returns Result(data=chunks) on AST parse success
- Returns Result(data=None, errors=[ErrorInfo]) on parse failure
Legacy _chunk_code calls helper; on Result errors, falls back to
_chunk_text (preserving original behavior). The catch logic is in the
legacy, not the helper, so the caller decides the fallback strategy.
Audit: rag_engine BC 4 -> 3.
This commit is contained in:
+24
-7
@@ -207,22 +207,39 @@ class RAGEngine:
|
|||||||
start += (chunk_size - overlap)
|
start += (chunk_size - overlap)
|
||||||
return chunks
|
return chunks
|
||||||
|
|
||||||
def _chunk_code(self, content: str, file_path: str) -> List[str]:
|
def _chunk_code_result(self, content: str, file_path: str) -> Result[List[str]]:
|
||||||
"""AST-aware chunking for Python code."""
|
"""AST-aware chunking for Python code. Returns Result[List[str]].
|
||||||
|
|
||||||
|
On AST parse failure, returns Result(errors=[ErrorInfo]). The legacy
|
||||||
|
caller (_chunk_code) decides whether to fallback to text chunking
|
||||||
|
(preserving the original behavior).
|
||||||
|
"""
|
||||||
try:
|
try:
|
||||||
parser = ASTParser("python")
|
parser = ASTParser("python")
|
||||||
tree = parser.parse(content)
|
tree = parser.parse(content)
|
||||||
chunks = []
|
chunks: List[str] = []
|
||||||
|
|
||||||
for node in tree.root_node.children:
|
for node in tree.root_node.children:
|
||||||
if node.type in ("function_definition", "class_definition"):
|
if node.type in ("function_definition", "class_definition"):
|
||||||
chunks.append(content[node.start_byte:node.end_byte])
|
chunks.append(content[node.start_byte:node.end_byte])
|
||||||
|
|
||||||
if not chunks or len(content) < self.config.chunk_size:
|
return Result(data=chunks)
|
||||||
return self._chunk_text(content)
|
except Exception as e:
|
||||||
return chunks
|
return Result(
|
||||||
except Exception:
|
data=None,
|
||||||
|
errors=[ErrorInfo(kind=ErrorKind.INTERNAL, message=f"AST chunking failed for {file_path}: {e}", source="rag_engine._chunk_code_result", original=e)],
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
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)
|
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 index_file(self, file_path: str):
|
def index_file(self, file_path: str):
|
||||||
"""Reads, chunks, and indexes a file into the vector store."""
|
"""Reads, chunks, and indexes a file into the vector store."""
|
||||||
|
|||||||
@@ -0,0 +1,33 @@
|
|||||||
|
"""Phase 13 site 2: _chunk_code Result migration.
|
||||||
|
|
||||||
|
Site 2 (BC at L224): the AST-aware chunking has a fallback to text chunking
|
||||||
|
on any failure:
|
||||||
|
try:
|
||||||
|
parser = ASTParser('python')
|
||||||
|
tree = parser.parse(content)
|
||||||
|
...
|
||||||
|
return chunks
|
||||||
|
except Exception:
|
||||||
|
return self._chunk_text(content)
|
||||||
|
|
||||||
|
Body: broad catch + fallback to a different implementation. Per Phase 11
|
||||||
|
anti-sliming, this is an empty-default fallback. Migrate to Result.
|
||||||
|
"""
|
||||||
|
import sys
|
||||||
|
sys.path.insert(0, ".")
|
||||||
|
|
||||||
|
|
||||||
|
def test_phase13_site2_chunk_code_result_exists():
|
||||||
|
import src.rag_engine
|
||||||
|
assert hasattr(src.rag_engine.RAGEngine, "_chunk_code_result") or \
|
||||||
|
hasattr(src.rag_engine, "_chunk_code_result"), \
|
||||||
|
"_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
|
||||||
|
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'"
|
||||||
Reference in New Issue
Block a user