Studio: tighten RAG code comments
Shorten and condense comments across the RAG backend, frontend, and tests for readability. Comment text only; no code, strings, identifiers, or logic changed. License headers and lint/type pragmas are preserved.
This commit is contained in:
parent
2af60c5480
commit
d1348cac3f
60 changed files with 541 additions and 631 deletions
|
|
@ -18,7 +18,7 @@ def isolated_bm25_root(tmp_path, monkeypatch):
|
|||
from utils.paths import storage_roots
|
||||
|
||||
monkeypatch.setenv("UNSLOTH_STUDIO_HOME", str(tmp_path))
|
||||
# Reset module-level cache between tests.
|
||||
# Reset module cache between tests.
|
||||
from core.rag import bm25
|
||||
|
||||
bm25._cache.clear()
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ def test_chunk_pages_splits_long_text():
|
|||
for chunk in chunks:
|
||||
assert (
|
||||
_wc_counter(chunk.text) <= 55
|
||||
) # max + small slack from atomic split granularity
|
||||
) # max + slack from atomic split granularity
|
||||
|
||||
|
||||
def test_chunk_pages_short_text_is_one_chunk():
|
||||
|
|
@ -70,7 +70,7 @@ def test_chunk_pages_no_empty_chunks():
|
|||
|
||||
|
||||
def test_chunk_pages_overlap_produces_repeated_tokens():
|
||||
# Build a list of unique numbered sentences so we can detect overlap.
|
||||
# Unique numbered sentences let us detect overlap.
|
||||
sentences = [f"sentence-{i}" for i in range(40)]
|
||||
text = " ".join(sentences)
|
||||
chunks = chunk_pages(
|
||||
|
|
@ -82,14 +82,13 @@ def test_chunk_pages_overlap_produces_repeated_tokens():
|
|||
if len(chunks) >= 2:
|
||||
first_tail_words = set(chunks[0].text.split()[-4:])
|
||||
second_head_words = set(chunks[1].text.split()[:4])
|
||||
# At least one word should appear in both
|
||||
# At least one shared word.
|
||||
assert first_tail_words & second_head_words
|
||||
|
||||
|
||||
def test_chunk_pages_splits_on_markdown_headings():
|
||||
# Phase 3A: heading separators take priority over paragraph breaks
|
||||
# so chunks start at section boundaries when the parser emits
|
||||
# Markdown.
|
||||
# Phase 3A: heading separators outrank paragraph breaks, so chunks
|
||||
# start at section boundaries for Markdown.
|
||||
md = (
|
||||
"# First Section\n\n"
|
||||
+ "alpha " * 30
|
||||
|
|
@ -104,7 +103,7 @@ def test_chunk_pages_splits_on_markdown_headings():
|
|||
overlap_tokens = 0,
|
||||
token_counter = _wc_counter,
|
||||
)
|
||||
# We expect multiple chunks and at least one to begin at a heading.
|
||||
# Expect multiple chunks, at least one starting at a heading.
|
||||
assert len(chunks) >= 2
|
||||
starts_at_heading = sum(
|
||||
1 for c in chunks if c.text.lstrip().startswith(("# ", "## "))
|
||||
|
|
|
|||
|
|
@ -37,7 +37,7 @@ def test_spans_index_back_to_full_doc_text():
|
|||
assert chunks
|
||||
assert len(chunks) == len(char_spans)
|
||||
for chunk, (start, end) in zip(chunks, char_spans):
|
||||
# The chunk text must be exactly the slice of full_doc it claims.
|
||||
# Chunk text must equal the full_doc slice it claims.
|
||||
assert full_doc[start:end] == chunk.text
|
||||
|
||||
|
||||
|
|
@ -54,7 +54,7 @@ def test_chunks_inherit_page_number_by_overlap():
|
|||
)
|
||||
pages_seen = {c.page_number for c in chunks}
|
||||
assert pages_seen <= {1, 2}
|
||||
# Both pages should contribute at least one chunk.
|
||||
# Each page contributes at least one chunk.
|
||||
assert 1 in pages_seen
|
||||
assert 2 in pages_seen
|
||||
|
||||
|
|
@ -72,7 +72,7 @@ def test_full_doc_joins_pages_with_blank_line_separator():
|
|||
)
|
||||
assert "first" in full_doc
|
||||
assert "second" in full_doc
|
||||
# The two pages must be separated by exactly one blank line.
|
||||
# Pages separated by exactly one blank line.
|
||||
assert "first\n\nsecond" in full_doc
|
||||
|
||||
|
||||
|
|
@ -80,7 +80,7 @@ def test_full_doc_joins_pages_with_blank_line_separator():
|
|||
def test_late_chunk_encode_returns_one_vector_per_span():
|
||||
pytest.importorskip("sentence_transformers")
|
||||
pytest.importorskip("torch")
|
||||
# all-MiniLM-L6-v2 is ~80MB and embeds at 384 dims.
|
||||
# all-MiniLM-L6-v2: ~80MB, 384 dims.
|
||||
import os
|
||||
|
||||
os.environ.setdefault(
|
||||
|
|
@ -100,7 +100,7 @@ def test_late_chunk_encode_returns_one_vector_per_span():
|
|||
"# Results\n\n"
|
||||
"Accuracy improved by 12% over the baseline."
|
||||
)
|
||||
# char_spans for three chunks — one per section, picked manually.
|
||||
# char_spans: one per section, picked manually.
|
||||
char_spans = [
|
||||
(doc_text.index("The quick"), doc_text.index("\n\n# Methods")),
|
||||
(doc_text.index("We trained"), doc_text.index("\n\n# Results")),
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ def test_html_parser_returns_images_when_requested(tmp_path):
|
|||
pytest.importorskip("markdownify")
|
||||
from core.rag.parsers import parse
|
||||
|
||||
# A tiny 1x1 transparent PNG.
|
||||
# 1x1 transparent PNG.
|
||||
png_bytes = (
|
||||
b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01"
|
||||
b"\x08\x06\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\rIDATx\x9cc\xfc\xff"
|
||||
|
|
@ -57,12 +57,12 @@ def test_multimodal_late_combo_validator():
|
|||
|
||||
from routes.rag import _validate_mode_combo
|
||||
|
||||
# Allowed combos return None.
|
||||
# Allowed combos → None.
|
||||
assert _validate_mode_combo("text", "standard") is None
|
||||
assert _validate_mode_combo("text", "late") is None
|
||||
assert _validate_mode_combo("multimodal", "standard") is None
|
||||
|
||||
# Forbidden combo raises 400.
|
||||
# Forbidden combo → 400.
|
||||
with pytest.raises(HTTPException) as excinfo:
|
||||
_validate_mode_combo("multimodal", "late")
|
||||
assert excinfo.value.status_code == 400
|
||||
|
|
@ -76,7 +76,7 @@ def test_rag_embedder_matrix_excludes_multimodal_late():
|
|||
assert ("text", "late") in RAG_EMBEDDER_MATRIX
|
||||
assert ("multimodal", "standard") in RAG_EMBEDDER_MATRIX
|
||||
|
||||
# Unknown combos fall back to the legacy default rather than KeyError.
|
||||
# Unknown combos fall back to the legacy default, not KeyError.
|
||||
fallback = resolve_embedder("multimodal", "late")
|
||||
assert isinstance(fallback, str) and fallback
|
||||
|
||||
|
|
@ -101,7 +101,7 @@ def test_multimodal_encode_image_returns_vector(tmp_path, monkeypatch):
|
|||
pytest.importorskip("sentence_transformers")
|
||||
pytest.importorskip("PIL")
|
||||
monkeypatch.setenv("UNSLOTH_RAG_EMBEDDING_MODEL", "BAAI/BGE-VL-base")
|
||||
# Reset the embedder singleton so the env var takes effect.
|
||||
# Reset the embedder singleton so the env var applies.
|
||||
from core.rag import embeddings as embeddings_module
|
||||
|
||||
embeddings_module._model = None
|
||||
|
|
@ -121,7 +121,6 @@ def test_multimodal_encode_image_returns_vector(tmp_path, monkeypatch):
|
|||
dim = vectors[0].shape[0]
|
||||
assert dim > 0
|
||||
|
||||
# Text from the same model should also be `dim`-d — shared space is
|
||||
# the whole point of multimodal embedders.
|
||||
# Text shares the same dim — the point of a multimodal embedder.
|
||||
text_vec = embeddings_module.encode(["a red square"])[0]
|
||||
assert text_vec.shape[0] == dim
|
||||
|
|
|
|||
|
|
@ -39,14 +39,13 @@ def test_multimodal_subprocess_emits_image_and_caption_chunks(
|
|||
pytest.importorskip("PIL")
|
||||
pytest.importorskip("torch")
|
||||
|
||||
# Use a tmp studio root so the ingest subprocess writes images
|
||||
# somewhere isolated.
|
||||
# tmp studio root isolates the subprocess's image writes.
|
||||
monkeypatch.setenv("UNSLOTH_STUDIO_HOME", str(tmp_path))
|
||||
monkeypatch.setenv("UNSLOTH_RAG_EMBEDDING_MODEL", "BAAI/BGE-VL-base")
|
||||
monkeypatch.setenv("UNSLOTH_RAG_CHUNK_SIZE", "200")
|
||||
monkeypatch.setenv("UNSLOTH_RAG_CHUNK_OVERLAP", "20")
|
||||
|
||||
# Reset module-level caches so the new env vars take effect.
|
||||
# Reset module caches so the new env vars apply.
|
||||
import importlib
|
||||
|
||||
import utils.rag.config as rag_config
|
||||
|
|
@ -57,7 +56,7 @@ def test_multimodal_subprocess_emits_image_and_caption_chunks(
|
|||
embeddings_module._model = None
|
||||
embeddings_module._model_name = None
|
||||
|
||||
# Generate a small PDF with text + one embedded image.
|
||||
# Small PDF: text + one embedded image.
|
||||
from PIL import Image
|
||||
|
||||
img = Image.new("RGB", (96, 64), (200, 100, 50))
|
||||
|
|
@ -83,7 +82,7 @@ def test_multimodal_subprocess_emits_image_and_caption_chunks(
|
|||
doc.save(str(pdf_path))
|
||||
doc.close()
|
||||
|
||||
# Drive the subprocess worker in-process with a regular queue.
|
||||
# Drive the worker in-process via a regular queue.
|
||||
from core.rag.ingestion import _subprocess_worker
|
||||
|
||||
out_queue: "queue_module.Queue[dict]" = queue_module.Queue()
|
||||
|
|
@ -99,19 +98,18 @@ def test_multimodal_subprocess_emits_image_and_caption_chunks(
|
|||
document_id = "test-doc-1",
|
||||
)
|
||||
|
||||
# Drain everything (the queue is in-process so order is stable).
|
||||
# Drain all events (in-process queue, stable order).
|
||||
events: list[dict] = []
|
||||
while not out_queue.empty():
|
||||
events.append(out_queue.get_nowait())
|
||||
|
||||
# The worker must emit at least one chunks_batch and exactly one
|
||||
# terminal complete/error event.
|
||||
# Expect >=1 chunks_batch and exactly one terminal complete/error.
|
||||
assert any(e["type"] == "chunks_batch" for e in events)
|
||||
terminals = [e for e in events if e["type"] in ("complete", "error")]
|
||||
assert len(terminals) == 1, terminals
|
||||
assert terminals[0]["type"] == "complete"
|
||||
|
||||
# Collect all chunks across batches.
|
||||
# Collect chunks across batches.
|
||||
all_chunks: list[dict] = []
|
||||
for e in events:
|
||||
if e["type"] == "chunks_batch":
|
||||
|
|
@ -120,12 +118,10 @@ def test_multimodal_subprocess_emits_image_and_caption_chunks(
|
|||
kinds = [c.get("kind") for c in all_chunks]
|
||||
assert "text" in kinds, "expected at least one text chunk"
|
||||
assert "image" in kinds, "expected at least one image chunk"
|
||||
# The PDF has a paragraph immediately after the image, so caption
|
||||
# pairing should fire.
|
||||
# Paragraph right after the image triggers caption pairing.
|
||||
assert "caption" in kinds, "expected at least one caption chunk"
|
||||
|
||||
# Image chunks must carry a file path that exists on disk under
|
||||
# the tmp studio root.
|
||||
# Image chunks carry a path on disk under the tmp studio root.
|
||||
image_chunks = [c for c in all_chunks if c.get("kind") == "image"]
|
||||
for chunk in image_chunks:
|
||||
assert chunk.get("image_path"), chunk
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ def test_markdown_parser_preserves_headings(tmp_path):
|
|||
file.write_text("# Title\n\nBody text with **emphasis**.", encoding = "utf-8")
|
||||
result = parse(file)
|
||||
assert result.pages
|
||||
# Markdown should pass through unchanged — heading marker preserved.
|
||||
# Markdown passes through; heading marker preserved.
|
||||
assert "# Title" in result.pages[0].text
|
||||
|
||||
|
||||
|
|
@ -67,7 +67,7 @@ def test_html_parser_emits_markdown_headings(tmp_path):
|
|||
result = parse(file)
|
||||
assert result.pages
|
||||
md = result.pages[0].text
|
||||
# markdownify converts <h1> → '# ', <h2> → '## '
|
||||
# markdownify: <h1> → '# ', <h2> → '## '
|
||||
assert "# Main Title" in md
|
||||
assert "## Sub Section" in md
|
||||
assert "visible text" in md
|
||||
|
|
@ -91,8 +91,7 @@ def test_pdf_parser_extracts_pages(tmp_path):
|
|||
with open(file, "wb") as f:
|
||||
writer.write(f)
|
||||
|
||||
# Blank page yields no extractable text — should return empty pages
|
||||
# without error.
|
||||
# Blank page: no text, returns empty pages without error.
|
||||
result = parse(file)
|
||||
assert isinstance(result.pages, list)
|
||||
assert isinstance(result.images, list)
|
||||
|
|
@ -116,7 +115,7 @@ def test_docx_parser_emits_markdown_headings(tmp_path):
|
|||
result = parse(file)
|
||||
assert result.pages
|
||||
md = result.pages[0].text
|
||||
# mammoth via _STYLE_MAP maps Heading 1/2 → h1/h2 → '# '/'## '.
|
||||
# mammoth _STYLE_MAP: Heading 1/2 → h1/h2 → '# '/'## '.
|
||||
assert "# Top Level Heading" in md
|
||||
assert "## Sub Heading" in md
|
||||
assert "First paragraph" in md
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ def test_rrf_fuses_two_rankings():
|
|||
dense = [Hit("c", 0.9), Hit("b", 0.8), Hit("d", 0.5)]
|
||||
fused = _rrf_fuse([bm25, dense], rrf_k = 60, top_k = 3)
|
||||
ids = [h.chunk_id for h in fused]
|
||||
# b appears at rank 2 in both -> highest fused score
|
||||
# b ranks 2 in both -> highest fused score.
|
||||
assert ids[0] == "b"
|
||||
assert set(ids) == {"a", "b", "c"} or set(ids) == {"b", "c", "a"}
|
||||
|
||||
|
|
@ -31,7 +31,7 @@ def test_rrf_top_k_limits_output():
|
|||
|
||||
|
||||
def test_rrf_unique_ranking():
|
||||
# Single ranking — fused order matches input order.
|
||||
# Single ranking: fused order matches input.
|
||||
ranking = [Hit("x", 0.0), Hit("y", 0.0), Hit("z", 0.0)]
|
||||
fused = _rrf_fuse([ranking], rrf_k = 60, top_k = 3)
|
||||
assert [h.chunk_id for h in fused] == ["x", "y", "z"]
|
||||
|
|
@ -41,6 +41,6 @@ def test_rrf_preserves_payload_from_first_ranking():
|
|||
a = Hit("a", 1.0, document_id = "doc1", chunk_index = 5)
|
||||
b = Hit("a", 2.0, document_id = "doc2", chunk_index = 7)
|
||||
fused = _rrf_fuse([[a], [b]], rrf_k = 60, top_k = 1)
|
||||
# First sighting wins for payload (deterministic)
|
||||
# First sighting wins for payload (deterministic).
|
||||
assert fused[0].document_id == "doc1"
|
||||
assert fused[0].chunk_index == 5
|
||||
|
|
|
|||
|
|
@ -127,7 +127,7 @@ def test_format_hits_produces_fenced_chunks():
|
|||
assert 'tokens="42"' in result
|
||||
assert "first body\n</chunk>" in result
|
||||
assert '<chunk id="2" source="beta.md" score="0.610">' in result
|
||||
# Blocks separated by a blank line so the model can scan the list.
|
||||
# Blank line between blocks so the model can scan them.
|
||||
assert "</chunk>\n\n<chunk" in result
|
||||
|
||||
|
||||
|
|
@ -188,8 +188,7 @@ def test_tool_spec_shape_is_openai_compatible():
|
|||
assert fn["name"] == "search_knowledge_base"
|
||||
assert "query" in fn["parameters"]["required"]
|
||||
assert "top_k" in fn["parameters"]["properties"]
|
||||
# Description should hint at when to call so the LLM picks it up
|
||||
# appropriately. Don't lock the exact wording.
|
||||
# Description hints when to call; don't lock the exact wording.
|
||||
assert "documents" in fn["description"].lower()
|
||||
|
||||
|
||||
|
|
@ -263,5 +262,5 @@ def test_all_tools_includes_rag():
|
|||
|
||||
names = [t["function"]["name"] for t in ALL_TOOLS]
|
||||
assert "search_knowledge_base" in names
|
||||
assert "web_search" in names # regression — we shouldn't have removed the others
|
||||
assert "web_search" in names # regression: others must stay
|
||||
assert "python" in names
|
||||
|
|
|
|||
|
|
@ -49,7 +49,7 @@ def test_upsert_and_search_returns_nearest_first(isolated_rag_db):
|
|||
results = vector_store.search(scope, [1.0, 0.0, 0.0, 0.0], top_k = 2)
|
||||
assert len(results) == 2
|
||||
assert results[0]["chunk_id"] == "p1"
|
||||
# Cosine similarity converted to [0, 1]; closer = higher.
|
||||
# Cosine mapped to [0, 1]; closer = higher.
|
||||
assert results[0]["score"] > results[1]["score"]
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue