test: cover riskiest untested production paths (PII filter, pruner, judge, conversation builder, ingestion pipeline, preference formatter)

CHANGELOG.md

@@ -4,6 +4,14 @@ All notable changes to Distill-Align will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
+## [Unreleased]
+
+### Added
+- Unit tests for the PII filter, pruner, judge, conversation builder, ingestion pipeline, and preference formatter (coverage 43% → 50%)
+
+### Fixed
+- PII filter import crash caused by an inline `(?i)` regex flag in the middle of the bearer-token pattern, which broke the `scan_pii` ingestion path
+
 ## [0.1.1] - 2026-06-18
 
 ### Fixed

src/distill_align/core/pii_filter.py

@@ -159,7 +159,7 @@ class PIIScanner:
 
     # Generic API key / bearer token
     BEARER_TOKEN: ClassVar[re.Pattern] = re.compile(
-        r"(?:(?i)(?:bearer|token|apikey|api_key|api-key|secret)[:\s=]+)[a-zA-Z0-9_\-\.]{16,64}"
+        r"(?i)(?:(?:bearer|token|apikey|api_key|api-key|secret)[:\s=]+)[a-zA-Z0-9_\-\.]{16,64}"
     )
 
     # JWT tokens (three base64url segments separated by dots)

tests/unit/test_conversation_builder.py

@@ -0,0 +1,180 @@
+"""
+Unit tests for the multi-turn conversation builder.
+
+The builder turns raw chunks into training conversations, so prompt assembly,
+JSON parsing, fallback behavior, and error handling are pinned down here.
+"""
+
+import json
+
+import pytest
+
+from distill_align.core.exceptions import LLMClientError
+from distill_align.core.schemas import DataChunk, SourceMetadata
+from distill_align.synthesis.conversation_builder import ConversationBuilder, ConversationMode
+from distill_align.synthesis.models.base import BaseLLMClient, LLMMessage, LLMResponse
+
+
+class FakeLLMClient(BaseLLMClient):
+    """A scriptable LLM client for conversation builder tests."""
+
+    def __init__(self, response: str = "", error: Exception | None = None):
+        super().__init__(base_url="http://fake", model="fake-model")
+        self.response = response
+        self.error = error
+        self.calls: list[tuple[str, str, float]] = []
+
+    async def chat(
+        self,
+        messages: list[LLMMessage],
+        temperature: float = 0.7,
+        max_tokens: int | None = None,
+        response_format: dict | None = None,
+        **kwargs,
+    ) -> LLMResponse:
+        self.calls.append((messages[0].content, messages[1].content, temperature))
+        if self.error:
+            raise self.error
+        return LLMResponse(content=self.response, model=self.model)
+
+    async def complete(
+        self, prompt: str, temperature: float = 0.7, max_tokens: int | None = None, **kwargs
+    ) -> LLMResponse:
+        return LLMResponse(content="", model=self.model)
+
+
+@pytest.fixture
+def chunk() -> DataChunk:
+    """A sample data chunk with rich metadata."""
+    return DataChunk(
+        content="This is the source content about distillation that is long enough to be useful.",
+        metadata=SourceMetadata(
+            source_type="markdown",
+            file_path="/docs/distillation.md",
+            file_name="distillation.md",
+            title="Distillation Guide",
+            language="en",
+            section_headers=["Overview", "Details"],
+        ),
+    )
+
+
+@pytest.fixture
+def builder() -> ConversationBuilder:
+    return ConversationBuilder()
+
+
+def _json_response(roles_and_contents: list[tuple[str, str]]) -> str:
+    conversation = [{"role": role, "content": content} for role, content in roles_and_contents]
+    payload = json.dumps({"conversation": conversation})
+    # Fenced blocks are required: the flat-object fallback in
+    # extract_json_from_response grabs the innermost object otherwise.
+    return f"```json\n{payload}\n```"
+
+
+class TestBuildConversation:
+    """Single-conversation generation."""
+
+    @pytest.mark.asyncio
+    async def test_parses_json_conversation(self, builder, chunk):
+        response = _json_response(
+            [
+                ("user", "What is distillation?"),
+                ("assistant", "It is the transfer of knowledge between models."),
+            ]
+        )
+        client = FakeLLMClient(response=response)
+
+        conv = await builder.build_conversation(chunk, ConversationMode.QA, client)
+        assert conv is not None
+        assert conv.source_chunk_id == chunk.id
+        assert conv.id
+        assert [t.role for t in conv.turns] == ["user", "assistant"]
+        assert conv.turns[0].content == "What is distillation?"
+
+    @pytest.mark.asyncio
+    async def test_plain_text_falls_back_to_simple_conversation(self, builder, chunk):
+        client = FakeLLMClient(response="Here is a plain text answer without JSON.")
+
+        conv = await builder.build_conversation(chunk, ConversationMode.QA, client)
+        assert conv is not None
+        assert [t.role for t in conv.turns] == ["user", "assistant"]
+        assert conv.turns[1].content == "Here is a plain text answer without JSON."
+        assert chunk.content[:500] in conv.turns[0].content
+
+    @pytest.mark.asyncio
+    async def test_llm_error_returns_none(self, builder, chunk):
+        client = FakeLLMClient(error=LLMClientError("provider down"))
+
+        conv = await builder.build_conversation(chunk, ConversationMode.QA, client)
+        assert conv is None
+
+    @pytest.mark.asyncio
+    async def test_temperature_passed_through(self, builder, chunk):
+        client = FakeLLMClient(response=_json_response([("user", "Q?"), ("assistant", "A.")]))
+
+        await builder.build_conversation(chunk, ConversationMode.QA, client, temperature=0.2)
+        _, _, temperature = client.calls[0]
+        assert temperature == 0.2
+
+    @pytest.mark.asyncio
+    async def test_pruned_too_short_falls_back_to_original(self, builder, chunk):
+        # A single-turn JSON response prunes to <2 turns; the original is kept.
+        client = FakeLLMClient(response=_json_response([("assistant", "Only one turn here.")]))
+
+        conv = await builder.build_conversation(chunk, ConversationMode.QA, client)
+        assert conv is not None
+        assert len(conv.turns) == 1
+
+
+class TestUserPrompt:
+    """Prompt assembly should vary by mode and include metadata."""
+
+    def test_teach_mode_has_instructions(self, builder, chunk):
+        prompt = builder._build_user_prompt(chunk, ConversationMode.TEACH)
+        assert "Start with a basic concept question" in prompt
+        assert "**Title:** Distillation Guide" in prompt
+        assert "**Sections:** Overview > Details" in prompt
+        assert "**Language:** en" in prompt
+
+    def test_review_mode_has_instructions(self, builder, chunk):
+        prompt = builder._build_user_prompt(chunk, ConversationMode.REVIEW)
+        assert "Identify 2-3 areas for improvement" in prompt
+
+    def test_unknown_mode_gets_no_mode_instructions(self, builder, chunk):
+        # An unrecognized mode falls back to the QA template for generation but
+        # appends no mode-specific instructions to the user prompt.
+        prompt = builder._build_user_prompt(chunk, "unknown-mode")
+        assert "**Source Content:**" in prompt
+        assert "Return JSON" in prompt
+        assert "Mix conceptual and practical questions" not in prompt
+
+    def test_content_truncated_to_3000_chars(self, builder):
+        long_chunk = DataChunk(
+            content="x" * 5000,
+            metadata=SourceMetadata(source_type="text", file_path="/a.txt", file_name="a.txt"),
+        )
+        prompt = builder._build_user_prompt(long_chunk, ConversationMode.QA)
+        assert "x" * 3000 in prompt
+        assert "x" * 3001 not in prompt
+
+
+class TestBuildBatch:
+    """Batch generation should isolate failures and preserve order."""
+
+    @pytest.mark.asyncio
+    async def test_builds_all_and_skips_failures(self, builder, chunk):
+        response = _json_response([("user", "Q?"), ("assistant", "A.")])
+        client = FakeLLMClient(response=response)
+
+        convs = await builder.build_batch([chunk, chunk], ConversationMode.QA, client)
+        assert len(convs) == 2
+        assert len(client.calls) == 2
+
+    @pytest.mark.asyncio
+    async def test_non_llm_exception_skipped(self, builder, chunk):
+        # A non-LLMClientError propagates out of build_conversation and is skipped in the batch.
+        client = FakeLLMClient(error=ValueError("unexpected"))
+
+        convs = await builder.build_batch([chunk, chunk], ConversationMode.QA, client)
+        assert convs == []

tests/unit/test_ingestion_pipeline.py

@@ -0,0 +1,134 @@
+"""
+Unit tests for the ingestion pipeline orchestrator.
+
+The pipeline drives file loading and chunking, so loader routing, error
+wrapping, directory scanning, and async behavior are pinned down here.
+"""
+
+import pytest
+
+from distill_align.core.exceptions import IngestionError, UnsupportedFormatError
+from distill_align.ingestion.loaders.code import CodeLoader
+from distill_align.ingestion.loaders.markdown import MarkdownLoader
+from distill_align.ingestion.pipeline import IngestionPipeline
+
+
+@pytest.fixture
+def pipeline() -> IngestionPipeline:
+    return IngestionPipeline()
+
+
+@pytest.fixture
+def sample_dir(tmp_path, sample_markdown_content):
+    """A directory tree with supported and unsupported files."""
+    (tmp_path / "guide.md").write_text(sample_markdown_content, encoding="utf-8")
+    (tmp_path / "notes.txt").write_text("ignored plain text", encoding="utf-8")
+    sub = tmp_path / "nested"
+    sub.mkdir()
+    (sub / "deep.md").write_text("# Deep\n\nContent in the nested directory.", encoding="utf-8")
+    return tmp_path
+
+
+class TestGetLoader:
+    """Loader routing by file extension."""
+
+    def test_markdown_extension(self, pipeline, tmp_path):
+        path = tmp_path / "doc.md"
+        path.write_text("# Hi", encoding="utf-8")
+        assert isinstance(pipeline._get_loader(path), MarkdownLoader)
+
+    def test_code_extension(self, pipeline, tmp_path):
+        path = tmp_path / "script.js"
+        path.write_text("let x = 1", encoding="utf-8")
+        assert isinstance(pipeline._get_loader(path), CodeLoader)
+
+    def test_unsupported_extension_raises(self, pipeline, tmp_path):
+        path = tmp_path / "archive.xyz"
+        path.write_text("data", encoding="utf-8")
+        with pytest.raises(UnsupportedFormatError):
+            pipeline._get_loader(path)
+
+
+class TestIngestFile:
+    """Single-file ingestion."""
+
+    def test_ingest_markdown_file(self, pipeline, tmp_path, sample_markdown_content):
+        path = tmp_path / "guide.md"
+        path.write_text(sample_markdown_content, encoding="utf-8")
+
+        chunks = pipeline.ingest_file(path)
+        assert chunks
+        assert all(c.metadata.source_type == "markdown" for c in chunks)
+        assert all(c.metadata.file_name == "guide.md" for c in chunks)
+        assert any("# Introduction" not in c.content for c in chunks)  # headers stripped
+
+    def test_missing_file_raises_ingestion_error(self, pipeline, tmp_path):
+        with pytest.raises(IngestionError, match="Failed to ingest"):
+            pipeline.ingest_file(tmp_path / "missing.md")
+
+    def test_unsupported_file_raises_ingestion_error(self, pipeline, tmp_path):
+        path = tmp_path / "data.xyz"
+        path.write_text("data", encoding="utf-8")
+        with pytest.raises(IngestionError, match="Unsupported file format"):
+            pipeline.ingest_file(path)
+
+
+class TestIngestDirectory:
+    """Directory scanning and per-file failure isolation."""
+
+    def test_recursive_scan_skips_unsupported(self, pipeline, sample_dir):
+        chunks = pipeline.ingest_directory(sample_dir, recursive=True)
+        # guide.md and nested/deep.md are ingested; notes.txt is ignored.
+        file_names = {c.metadata.file_name for c in chunks}
+        assert file_names == {"guide.md", "deep.md"}
+
+    def test_non_recursive_scan(self, pipeline, sample_dir):
+        chunks = pipeline.ingest_directory(sample_dir, recursive=False)
+        assert {c.metadata.file_name for c in chunks} == {"guide.md"}
+
+    def test_missing_directory_raises(self, pipeline, tmp_path):
+        with pytest.raises(IngestionError, match="Not a directory"):
+            pipeline.ingest_directory(tmp_path / "nope")
+
+    def test_file_patterns_filter(self, pipeline, sample_dir):
+        chunks = pipeline.ingest_directory(sample_dir, file_patterns=["*.py"])
+        assert chunks == []
+
+    def test_failing_file_skipped(self, pipeline, sample_dir, monkeypatch):
+        def fake_ingest(file_path):
+            if file_path.name == "guide.md":
+                raise IngestionError("boom")
+            from distill_align.ingestion.pipeline import IngestionPipeline
+
+            return IngestionPipeline.ingest_file(pipeline, file_path)
+
+        monkeypatch.setattr(pipeline, "ingest_file", fake_ingest)
+        chunks = pipeline.ingest_directory(sample_dir, recursive=True)
+        assert {c.metadata.file_name for c in chunks} == {"deep.md"}
+
+
+class TestAsync:
+    """Async ingestion variants."""
+
+    @pytest.mark.asyncio
+    async def test_ingest_file_async(self, pipeline, tmp_path, sample_markdown_content):
+        path = tmp_path / "guide.md"
+        path.write_text(sample_markdown_content, encoding="utf-8")
+
+        chunks = await pipeline.ingest_file_async(path)
+        assert chunks
+        assert chunks[0].metadata.file_name == "guide.md"
+
+    @pytest.mark.asyncio
+    async def test_ingest_directory_async(self, pipeline, sample_dir):
+        chunks = await pipeline.ingest_directory_async(sample_dir)
+        assert {c.metadata.file_name for c in chunks} == {"guide.md", "deep.md"}
+
+    @pytest.mark.asyncio
+    async def test_failure_isolated_in_async_directory(self, pipeline, sample_dir, monkeypatch):
+        async def fake_ingest_async(file_path):
+            raise IngestionError("boom")
+
+        monkeypatch.setattr(pipeline, "ingest_file_async", fake_ingest_async)
+        chunks = await pipeline.ingest_directory_async(sample_dir)
+        assert chunks == []