mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-07-28 14:07:55 +00:00
bc9fa6b3c8
Memory tools degraded silently and misleadingly when the mandatory
`openjarvis_rust` extension was absent from the *serving* venv:
- `POST /v1/memory/store` returned HTTP 200 `{"status":"stored","note":
"no backend available"}` and stored nothing (silent data loss).
- `POST /v1/memory/index` returned a generic "No memory backend available",
and the desktop frontend discarded the server `detail` and threw a blanket
"Failed to index path", blaming the path instead of the real cause.
- `GET /v1/memory/config` reported `backend_type: sqlite` even though no
backend could be constructed.
Root cause: `SQLiteMemory.__init__` calls `get_rust_module()` (which raises
ImportError by design — the Rust ext is mandatory, no Python fallback), and
`_get_memory_backend` swallowed that ImportError and returned `None`,
conflating "native extension missing" (a hard install error) with "memory
intentionally disabled" (benign). A chunking floor also silently dropped whole
short documents, and the installer never verified the extension imported from
the serving venv before writing its success marker.
Fix (no fake Python fallback — the Rust ext stays mandatory by design):
- Add `MemoryBackendUnavailable` + `RUST_MISSING_HINT` in tools/storage/_stubs.
`SQLiteMemory.__init__` translates the bridge ImportError into this clear,
actionable error ("run `uv run maturin develop ...`").
- `_get_memory_backend` distinguishes the two cases: a missing native ext
raises HTTP 503 with the actionable hint; a benign unconfigured backend
still returns `None` (graceful path preserved for search/stats).
- `/store` now returns 503 instead of a 200 silent no-op.
- `/config` reports `available: false` + `detail` instead of falsely claiming
a healthy `backend_type`.
- `/index` adds a `note` when `chunks_indexed == 0` so "indexed" never
silently means "stored nothing".
- chunk_text no longer drops an entire short document below `min_chunk_size`
(the floor only discards tiny *trailing* fragments now).
- Frontend `storeMemory`/`indexMemoryPath` surface the server `detail` instead
of blanket strings; `MemoryConfig` gains optional `available`/`detail`.
(Left the pre-existing `backend` vs `backend_type` mismatch untouched.)
- build-extension.sh verifies `import openjarvis_rust` succeeds in the serving
venv before writing the `extension-built` marker.
Regression tests: tests/server/test_api_routes.py::TestMemoryRustMissing mocks
`get_rust_module` to raise ImportError and asserts /store (503, not 200 no-op),
/index (actionable detail, not "Failed to index path"), and /config
(available:false) all surface the clear error; tests/memory/test_chunking.py
asserts short-only docs are kept while tiny trailing fragments are still
filtered.
Fixes #502
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
120 lines
4.0 KiB
Python
120 lines
4.0 KiB
Python
"""Tests for the document chunking pipeline."""
|
|
|
|
from __future__ import annotations
|
|
|
|
from openjarvis.tools.storage.chunking import ChunkConfig, chunk_text
|
|
|
|
|
|
def test_empty_string_returns_empty():
|
|
assert chunk_text("") == []
|
|
|
|
|
|
def test_whitespace_only_returns_empty():
|
|
assert chunk_text(" \n\n ") == []
|
|
|
|
|
|
def test_short_text_single_chunk():
|
|
# Need >= 50 words (default min_chunk_size)
|
|
words = [f"word{i}" for i in range(60)]
|
|
text = " ".join(words)
|
|
chunks = chunk_text(text, source="test.txt")
|
|
assert len(chunks) == 1
|
|
assert chunks[0].source == "test.txt"
|
|
assert chunks[0].index == 0
|
|
assert "word0" in chunks[0].content
|
|
|
|
|
|
def test_long_text_multiple_chunks():
|
|
# Build text that exceeds 512 tokens
|
|
words = [f"word{i}" for i in range(600)]
|
|
text = " ".join(words)
|
|
chunks = chunk_text(text)
|
|
assert len(chunks) >= 2
|
|
|
|
|
|
def test_chunk_overlap():
|
|
cfg = ChunkConfig(chunk_size=100, chunk_overlap=20, min_chunk_size=5)
|
|
words = [f"w{i}" for i in range(250)]
|
|
text = " ".join(words)
|
|
chunks = chunk_text(text, config=cfg)
|
|
assert len(chunks) >= 2
|
|
|
|
# The end of chunk 0 and start of chunk 1 should overlap
|
|
first_tokens = chunks[0].content.split()
|
|
second_tokens = chunks[1].content.split()
|
|
tail = first_tokens[-20:]
|
|
head = second_tokens[:20]
|
|
assert tail == head
|
|
|
|
|
|
def test_paragraph_boundary_respected():
|
|
cfg = ChunkConfig(chunk_size=20, chunk_overlap=0, min_chunk_size=3)
|
|
para1 = " ".join(f"a{i}" for i in range(10))
|
|
para2 = " ".join(f"b{i}" for i in range(10))
|
|
text = f"{para1}\n\n{para2}"
|
|
chunks = chunk_text(text, config=cfg)
|
|
# Both paragraphs fit in one chunk (10 + 10 = 20 <= 20)
|
|
assert len(chunks) == 1
|
|
|
|
|
|
def test_custom_config():
|
|
cfg = ChunkConfig(chunk_size=50, chunk_overlap=10, min_chunk_size=5)
|
|
words = [f"tok{i}" for i in range(200)]
|
|
text = " ".join(words)
|
|
chunks = chunk_text(text, config=cfg)
|
|
# Should produce multiple chunks
|
|
assert len(chunks) >= 3
|
|
|
|
|
|
def test_short_only_document_not_dropped():
|
|
"""A whole document below min_chunk_size must still produce a chunk.
|
|
|
|
Regression for #502 follow-up: previously a folder of short notes indexed
|
|
to ``chunks_indexed: 0`` (HTTP 200), silently storing nothing. ``min_chunk_size``
|
|
should only discard tiny *trailing fragments*, never an entire short doc.
|
|
"""
|
|
cfg = ChunkConfig(chunk_size=100, chunk_overlap=0, min_chunk_size=50)
|
|
# 30 words is below min_chunk_size=50, but it's the entire document.
|
|
words = [f"w{i}" for i in range(30)]
|
|
text = " ".join(words)
|
|
chunks = chunk_text(text, config=cfg)
|
|
assert len(chunks) == 1
|
|
assert chunks[0].content == text
|
|
|
|
|
|
def test_short_real_world_note_not_dropped():
|
|
"""The exact repro from the issue: a ~4-word note must not vanish."""
|
|
chunks = chunk_text("hello world\nsome content\n", source="a.txt")
|
|
assert len(chunks) == 1
|
|
assert "hello world" in chunks[0].content
|
|
|
|
|
|
def test_min_chunk_size_filters_tiny_trailing_fragment():
|
|
"""A tiny fragment trailing a real chunk is still dropped by the floor."""
|
|
cfg = ChunkConfig(chunk_size=50, chunk_overlap=0, min_chunk_size=10)
|
|
# Two paragraphs: the first fills a real chunk, the second is a tiny tail.
|
|
para1 = " ".join(f"a{i}" for i in range(50))
|
|
para2 = " ".join(f"b{i}" for i in range(3)) # 3 words < min_chunk_size=10
|
|
text = f"{para1}\n\n{para2}"
|
|
chunks = chunk_text(text, config=cfg)
|
|
# The 3-word trailing fragment is discarded; only the real chunk remains.
|
|
assert len(chunks) == 1
|
|
assert "b0" not in chunks[0].content
|
|
|
|
|
|
def test_source_propagated():
|
|
words = [f"word{i}" for i in range(60)]
|
|
text = " ".join(words)
|
|
chunks = chunk_text(text, source="myfile.md")
|
|
assert len(chunks) == 1
|
|
assert chunks[0].source == "myfile.md"
|
|
|
|
|
|
def test_chunk_index_sequential():
|
|
cfg = ChunkConfig(chunk_size=50, chunk_overlap=0, min_chunk_size=5)
|
|
words = [f"w{i}" for i in range(200)]
|
|
text = " ".join(words)
|
|
chunks = chunk_text(text, config=cfg)
|
|
for i, chunk in enumerate(chunks):
|
|
assert chunk.index == i
|