mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(rag): close audit gaps in the in-house engine
An adversarial audit of the piragi -> in-house swap surfaced nine confirmed issues; this fixes all of them. - Re-ingest now REPLACES a source's chunks instead of appending. Add VectorStore.delete_by_source and BaseIndexPlugin.replace_on_reingest (default True), called before add_chunks in both ingest paths. Without it every startup / periodic / manual reindex appended a fresh copy of each doc's chunks, growing the tables unbounded and crowding out distinct results. Conversations opt OUT (replace_on_reingest=False): their many messages share one source URI, so delete-by-source would wipe history. - index_* now honor the plugin IngestResult. The explicit record endpoints (error / standard / decision / review / learning) raise on failure instead of writing a green tracking row for content that never persisted; conversation / journal indexing stays best-effort but skips the tracking row when the embed fails. index_message / index_entry return IngestResult. - A deprecated index type (code) now returns 404 instead of a 500 leaked from _get_plugin's missing-plugin error: add OptimalService.is_index_registered and guard the stats / clear / refresh routes. The panel drops the dead 'Code' category, filter, badge, label, and mock data. - Panel: getContext reads 'results' (matches SearchResponse) instead of a non-existent 'context' field; the reindex toast no longer reports phantom '0 code files'; the stats 'Updated' label uses the max timestamp across indexes rather than indexes[0]; ProactiveContextItem matches the wire shape. - Drop the always-zero per-document chunk_count from the documents API. - Remove dead RAG settings (hybrid_search, cross_encoder) the engine never consumed, and correct stale piragi / BM25 references in code, README, and CLAUDE.md. Delete the unused duplicate roboco/kb embedder package the swap shipped. Adds tests for replace-on-reingest (incl. the conversations carve-out) and the deprecated-index 404.
This commit is contained in:
@@ -339,6 +339,7 @@ async def test_get_single_stats_invalid_type(optimal_client: AsyncClient) -> Non
|
||||
async def test_get_single_stats_success(optimal_client: AsyncClient) -> None:
|
||||
with patch("roboco.api.routes.optimal.get_optimal_service") as mock_get:
|
||||
mock_service = AsyncMock()
|
||||
mock_service.is_index_registered = MagicMock(return_value=True)
|
||||
mock_service.get_index_stats = AsyncMock(
|
||||
return_value={
|
||||
"index_type": "documentation",
|
||||
@@ -354,6 +355,20 @@ async def test_get_single_stats_success(optimal_client: AsyncClient) -> None:
|
||||
assert response.status_code == HTTPStatus.OK
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_single_stats_deprecated_index_returns_404(
|
||||
optimal_client: AsyncClient,
|
||||
) -> None:
|
||||
"""A valid-but-unregistered index type (e.g. deprecated `code`) returns 404,
|
||||
not a 500 leaked from _get_plugin's missing-plugin RuntimeError."""
|
||||
with patch("roboco.api.routes.optimal.get_optimal_service") as mock_get:
|
||||
mock_service = AsyncMock()
|
||||
mock_service.is_index_registered = MagicMock(return_value=False)
|
||||
mock_get.return_value = mock_service
|
||||
response = await optimal_client.get("/api/optimal/stats/code", headers=_HDR)
|
||||
assert response.status_code == HTTPStatus.NOT_FOUND
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_check_staleness_via_http(optimal_client: AsyncClient) -> None:
|
||||
"""`/stats/staleness` is now declared before `/stats/{index_type}`, so it
|
||||
@@ -450,6 +465,7 @@ async def test_clear_index_invalid_type(optimal_client: AsyncClient) -> None:
|
||||
async def test_clear_index_success(optimal_client: AsyncClient) -> None:
|
||||
with patch("roboco.api.routes.optimal.get_optimal_service") as mock_get:
|
||||
mock_service = AsyncMock()
|
||||
mock_service.is_index_registered = MagicMock(return_value=True)
|
||||
mock_service.clear_index = AsyncMock(return_value=None)
|
||||
mock_get.return_value = mock_service
|
||||
response = await optimal_client.delete(
|
||||
@@ -523,6 +539,7 @@ async def test_refresh_index_invalid_type(optimal_client: AsyncClient) -> None:
|
||||
async def test_refresh_index_with_sources(optimal_client: AsyncClient) -> None:
|
||||
with patch("roboco.api.routes.optimal.get_optimal_service") as mock_get:
|
||||
mock_service = AsyncMock()
|
||||
mock_service.is_index_registered = MagicMock(return_value=True)
|
||||
mock_service.refresh_index = AsyncMock(return_value=None)
|
||||
mock_get.return_value = mock_service
|
||||
response = await optimal_client.post(
|
||||
@@ -538,6 +555,7 @@ async def test_refresh_index_empty_sources(optimal_client: AsyncClient) -> None:
|
||||
"""Empty sources -> service.get_indexed_sources_for is called."""
|
||||
with patch("roboco.api.routes.optimal.get_optimal_service") as mock_get:
|
||||
mock_service = AsyncMock()
|
||||
mock_service.is_index_registered = MagicMock(return_value=True)
|
||||
mock_service.get_indexed_sources_for = AsyncMock(return_value=["x.md"])
|
||||
mock_service.refresh_index = AsyncMock(return_value=None)
|
||||
mock_get.return_value = mock_service
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
"""Re-ingesting a source must replace its chunks, not append duplicates.
|
||||
|
||||
Guards the HIGH fix: without a delete-by-source step, every startup/periodic/
|
||||
manual reindex appended a fresh copy of each doc's chunks, growing the tables
|
||||
unbounded and crowding out distinct results. The carve-out is conversations,
|
||||
whose many messages share one source URI — there, append must be preserved.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
from roboco.services.optimal_brain.indexes.conversations import ConversationsIndexPlugin
|
||||
from roboco.services.optimal_brain.indexes.standards import StandardsIndexPlugin
|
||||
from roboco.services.optimal_brain.text_chunker import Chunk, Document
|
||||
|
||||
|
||||
def _wire_plugin(
|
||||
plugin: object, source: str, monkeypatch: pytest.MonkeyPatch
|
||||
) -> AsyncMock:
|
||||
"""Attach mock store/chunker/embedder via monkeypatch; return the store mock."""
|
||||
chunk = Chunk(text="x" * 250, source=source, metadata={})
|
||||
store = AsyncMock()
|
||||
embedder = MagicMock()
|
||||
embedder.aembed_chunks = AsyncMock(return_value=[chunk])
|
||||
monkeypatch.setattr(plugin, "_store", store)
|
||||
monkeypatch.setattr(
|
||||
plugin, "_chunker", MagicMock(chunk_document=MagicMock(return_value=[chunk]))
|
||||
)
|
||||
monkeypatch.setattr(plugin, "_embedder", embedder)
|
||||
monkeypatch.setattr(plugin, "_initialized", True)
|
||||
return store
|
||||
|
||||
|
||||
def test_default_plugin_replaces_on_reingest() -> None:
|
||||
assert StandardsIndexPlugin.replace_on_reingest is True
|
||||
|
||||
|
||||
def test_conversations_plugin_appends_not_replaces() -> None:
|
||||
assert ConversationsIndexPlugin.replace_on_reingest is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reingest_deletes_existing_source_chunks_when_replacing(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
plugin = StandardsIndexPlugin()
|
||||
source = "roboco://standards/general/std-1"
|
||||
store = _wire_plugin(plugin, source, monkeypatch)
|
||||
doc = Document(content="x" * 250, source=source, metadata={})
|
||||
|
||||
count = await plugin._chunk_filter_embed_store(doc, {})
|
||||
|
||||
assert count == 1
|
||||
store.delete_by_source.assert_awaited_once_with(source)
|
||||
store.add_chunks.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reingest_preserves_history_for_conversations(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
plugin = ConversationsIndexPlugin()
|
||||
source = "roboco://conversations/sess-1-agent-1"
|
||||
store = _wire_plugin(plugin, source, monkeypatch)
|
||||
doc = Document(content="x" * 250, source=source, metadata={})
|
||||
|
||||
count = await plugin._chunk_filter_embed_store(doc, {})
|
||||
|
||||
assert count == 1
|
||||
store.delete_by_source.assert_not_awaited()
|
||||
store.add_chunks.assert_awaited_once()
|
||||
Reference in New Issue
Block a user