From 9fce8b7e39e09cb0de86eee134c1bd73c44f8fa2 Mon Sep 17 00:00:00 2001 From: Jer Miller Date: Tue, 14 Jul 2026 08:43:27 -0600 Subject: [PATCH] Route PDFs through document importer PDFs previously had multiple derivation paths: direct document import, text-transcript import, and the observe extraction path. That let the same source produce divergent stream layouts and dedup behavior.\n\nMake the importer boundary fail loudly if a PDF cannot resolve to the document file importer, remove PDF from generic text/audio classification, and drop the transcript reader's PDF branch. PDF imports now take file-importer preview and source-hash dedup consistently. --- solstone/think/importers/cli.py | 15 ++- solstone/think/importers/text.py | 13 +- tests/test_importer.py | 214 +++++++++++++++++++------------ tests/test_pdf_lazy_imports.py | 13 -- 4 files changed, 141 insertions(+), 114 deletions(-) diff --git a/solstone/think/importers/cli.py b/solstone/think/importers/cli.py index c6d0a3bc3..7aa7423f9 100644 --- a/solstone/think/importers/cli.py +++ b/solstone/think/importers/cli.py @@ -26,6 +26,7 @@ from solstone.think.importers.text import _read_transcript, process_transcript from solstone.think.importers.utils import save_import_segments from solstone.think.indexer.journal import index_file from solstone.think.journal_io import atomic_replace +from solstone.think.media import PDF_EXTENSIONS from solstone.think.segment import _touch_health_marker from solstone.think.streams import stream_name, update_stream, write_segment_stream from solstone.think.utils import ( @@ -542,7 +543,7 @@ def _import_one_from_args(args: argparse.Namespace) -> dict[str, Any] | None: # detection for image/structured imports. if _file_importer is None: _ext = os.path.splitext(args.media)[1].lower() - if _ext not in {".m4a", ".txt", ".md", ".pdf"}: + if _ext not in {".m4a", ".txt", ".md"}: from solstone.think.importers.file_importer import detect_file_importer detected = detect_file_importer(Path(args.media)) @@ -550,6 +551,12 @@ def _import_one_from_args(args: argparse.Namespace) -> dict[str, Any] | None: _file_importer = detected import_source = detected.name + if ( + _file_importer is None + and os.path.splitext(args.media)[1].lower() in PDF_EXTENSIONS + ): + raise ValueError(f"PDF import requires the document importer: {args.media}") + # Source-level dedup for audio/text (before timestamp detection and before # _setup_import rewrites args.media). Mirrors the file-importer dedup at the # file-importer branch; skipped for --force and --dry-run (which still preview). @@ -648,7 +655,7 @@ def _import_one_from_args(args: argparse.Namespace) -> dict[str, Any] | None: # reach the stream/manifest. File-importer sources are handled earlier. if import_source is None: _ext = os.path.splitext(args.media)[1].lower() - import_source = "text" if _ext in {".txt", ".md", ".pdf"} else "audio" + import_source = "text" if _ext in {".txt", ".md"} else "audio" stream = stream_name(import_source=import_source) needs_setup = _file_importer is None and not _is_in_imports(args.media) @@ -706,7 +713,7 @@ def _import_one_from_args(args: argparse.Namespace) -> dict[str, Any] | None: print(f" Target day: {day}") ext = os.path.splitext(args.media)[1].lower() - if ext in {".txt", ".md", ".pdf"}: + if ext in {".txt", ".md"}: from solstone.think.detect_transcript import detect_transcript_segment from solstone.think.importers.text import _time_to_seconds @@ -1132,7 +1139,7 @@ def _import_one_from_args(args: argparse.Namespace) -> dict[str, Any] | None: ) ) - elif ext in {".txt", ".md", ".pdf"}: + elif ext in {".txt", ".md"}: # Text transcript processing — no observe pipeline _set_stage("segmenting") diff --git a/solstone/think/importers/text.py b/solstone/think/importers/text.py index 3a8ce0f3e..3fa39cb54 100644 --- a/solstone/think/importers/text.py +++ b/solstone/think/importers/text.py @@ -11,28 +11,17 @@ from solstone.think.detect_transcript import ( detect_transcript_json, detect_transcript_segment, ) -from solstone.think.features import require_extra from solstone.think.importers.shared import write_segment logger = logging.getLogger(__name__) def _read_transcript(path: str) -> str: - """Return transcript text from a .txt/.md/.pdf file.""" + """Return transcript text from a .txt/.md file.""" ext = os.path.splitext(path)[1].lower() if ext in {".txt", ".md"}: with open(path, "r", encoding="utf-8") as f: return f.read() - if ext == ".pdf": - require_extra("pdf") - from pypdf import PdfReader - - reader = PdfReader(path) - parts = [] - for page in reader.pages: - text = page.extract_text() or "" - parts.append(text) - return "\n".join(parts) raise ValueError("unsupported transcript format") diff --git a/tests/test_importer.py b/tests/test_importer.py index 707a8dccb..27bcbf58b 100644 --- a/tests/test_importer.py +++ b/tests/test_importer.py @@ -24,6 +24,8 @@ from solstone.think.importers.file_importer import ImportPreview, ImportResult from solstone.think.importers.shared import install_source_file from solstone.think.utils import day_path +PDF_SENTINEL = "SOLSTONE_PDF_SENTINEL_DOCUMENT_ROUTING" + def _make_mock_file_importer(name="ics", display_name="ICS Calendar"): """Create a mock FileImporter for testing.""" @@ -77,6 +79,23 @@ def _configure_text_import_runtime(monkeypatch, mod): monkeypatch.setattr(mod, "_status_emitter", lambda: None) +def _write_text_pdf(path: Path, text: str) -> None: + pytest.importorskip("weasyprint") + pytest.importorskip("pypdf") + from weasyprint import HTML + + HTML(string=f"

Fixture

{text}

").write_pdf( + path + ) + + +def _configure_document_import_runtime(monkeypatch, mod, doc_mod, *, timestamp: float): + monkeypatch.setattr(mod, "CallosumConnection", lambda **kwargs: MagicMock()) + monkeypatch.setattr(mod, "_status_emitter", lambda: None) + monkeypatch.setattr(mod, "index_file", lambda *args, **kwargs: None) + monkeypatch.setattr(doc_mod, "_get_pdf_timestamp", lambda reader, path: timestamp) + + def _read_action_entries(journal_root: Path) -> list[dict]: """Read journal-level app action log entries for today.""" today = dt.datetime.now().strftime("%Y%m%d") @@ -656,109 +675,134 @@ def test_text_import_observed_events_are_batch_and_drain_once(tmp_path, monkeypa assert "20240101" in updated_days() -def test_importer_pdf(tmp_path, monkeypatch): - """Test importing a PDF transcript file.""" +def test_importer_pdf_routes_to_document_importer_and_dedups(tmp_path, monkeypatch): + """PDF imports use the document file importer, even with a bogus source hint.""" mod = importlib.import_module("solstone.think.importers.cli") - text_mod = importlib.import_module("solstone.think.importers.text") + doc_mod = importlib.import_module("solstone.think.importers.documents") + fixed_ts = dt.datetime(2025, 12, 5, 16, 30, 0).timestamp() - # Create a fake PDF file (content doesn't matter — pypdf is mocked) pdf = tmp_path / "meeting.pdf" - pdf.write_bytes(b"%PDF-1.4 fake") + _write_text_pdf(pdf, PDF_SENTINEL) monkeypatch.setenv("SOLSTONE_JOURNAL", str(tmp_path)) - monkeypatch.setattr( - mod, "detect_created", lambda p, **kw: {"day": "20251205", "time": "163000"} + _configure_document_import_runtime( + monkeypatch, + mod, + doc_mod, + timestamp=fixed_ts, ) - # Mock _read_transcript to return extracted text (bypasses pypdf) - monkeypatch.setattr( - text_mod, "_read_transcript", lambda path: "Board meeting notes\nAction items" + first = mod.import_one( + pdf, + timestamp="20251205_163000", + source="text", + wait_for_processing=False, ) - # Mock segment detection: single segment for short text - def mock_detect_segment(text, start_time): - return [("16:30:00", text)] + day_dir = day_path("20251205") + segment_dir = day_dir / "import.document" / "163000_0" + md_path = segment_dir / "document_transcript.md" + original_path = segment_dir / "original.pdf" + stream_json = segment_dir / "stream.json" + + assert first is not None + assert first["source_type"] == "document" + assert first["outputs"][0]["type"] == "file_import" + assert md_path.exists() + assert PDF_SENTINEL in md_path.read_text(encoding="utf-8") + assert original_path.exists() + assert not (day_dir / "import.text").exists() + assert json.loads(stream_json.read_text(encoding="utf-8"))["stream"] == ( + "import.document" + ) + + second = mod.import_one( + pdf, + timestamp="20251205_163100", + source="audio", + wait_for_processing=False, + ) - monkeypatch.setattr(text_mod, "detect_transcript_segment", mock_detect_segment) + assert second is not None + assert second["skipped"] is True + assert second["reason"] == "already_imported" + assert second["entry_count"] == 1 + assert isinstance(second["imported_at"], str) - # Mock JSON conversion - def mock_detect_json(text, segment_start): - return { - "entries": [ - { - "start": segment_start, - "speaker": "Jack", - "text": "Board meeting notes", - }, - {"start": "16:30:30", "speaker": "Ramon", "text": "Action items"}, - ], - "topics": "board meeting, action items", - "setting": "workplace", - } - monkeypatch.setattr(text_mod, "detect_transcript_json", mock_detect_json) +def test_importer_pdf_dry_run_uses_document_preview(tmp_path, monkeypatch, capsys): + mod = importlib.import_module("solstone.think.importers.cli") + doc_mod = importlib.import_module("solstone.think.importers.documents") + fixed_ts = dt.datetime(2025, 12, 5, 16, 30, 0).timestamp() + pdf = tmp_path / "meeting.pdf" + _write_text_pdf(pdf, PDF_SENTINEL) + + monkeypatch.setenv("SOLSTONE_JOURNAL", str(tmp_path)) + _configure_document_import_runtime( + monkeypatch, + mod, + doc_mod, + timestamp=fixed_ts, + ) + + result = mod.import_one(pdf, source="text", dry_run=True, json_output=True) + captured = capsys.readouterr() + preview = json.loads(captured.out.strip()) + + assert result is not None + assert result["dry_run"] is True + assert result["importer"] == "document" + assert preview["importer"] == "document" + assert preview["item_count"] == 1 + assert not (tmp_path / "chronicle").exists() + + +def test_importer_pdf_detection_none_raises_named_file(tmp_path, monkeypatch): + mod = importlib.import_module("solstone.think.importers.cli") + file_importer = importlib.import_module("solstone.think.importers.file_importer") + pdf = tmp_path / "unrouted.pdf" + pdf.write_bytes(b"%PDF-1.4 fake") + + monkeypatch.setenv("SOLSTONE_JOURNAL", str(tmp_path)) + monkeypatch.setattr(file_importer, "detect_file_importer", lambda path: None) + + with pytest.raises(ValueError) as exc: + mod.import_one( + pdf, + timestamp="20251205_163000", + wait_for_processing=False, + ) + + assert "PDF import requires the document importer" in str(exc.value) + assert str(pdf) in str(exc.value) + + +def test_corrupt_pdf_routes_to_document_importer_and_reports_error( + tmp_path, monkeypatch, capsys +): + mod = importlib.import_module("solstone.think.importers.cli") + corrupt = tmp_path / "corrupt.pdf" + corrupt.write_bytes(b"%PDF-1.4\nthis is not a real xref table\n%%EOF\n") - # Mock CallosumConnection and status emitter + monkeypatch.setenv("SOLSTONE_JOURNAL", str(tmp_path)) monkeypatch.setattr(mod, "CallosumConnection", lambda **kwargs: MagicMock()) monkeypatch.setattr(mod, "_status_emitter", lambda: None) - monkeypatch.setattr( - "sys.argv", - [ - "sol import", - str(pdf), - "--timestamp", - "20251205_163000", - "--facet", - "work", - "--setting", - "board meeting", - ], + result = mod.import_one( + corrupt, + timestamp="20251205_163000", + json_output=True, + wait_for_processing=False, ) - mod.main() - - day_dir = day_path("20251205") - # Single segment, last segment defaults to 5s - f1 = day_dir / "import.text" / "163000_5" / "conversation_transcript.jsonl" - assert f1.exists() + captured = capsys.readouterr() + output = json.loads(captured.out.strip().splitlines()[-1]) - lines = f1.read_text().strip().split("\n") - metadata = json.loads(lines[0]) - entries = [json.loads(line) for line in lines[1:]] - - # Verify metadata - assert metadata["imported"]["id"] == "20251205_163000" - assert metadata["imported"]["facet"] == "work" - assert metadata["imported"]["setting"] == "board meeting" - assert metadata["raw"] == "../../../imports/20251205_163000/meeting.pdf" - - # Verify entries — timestamps are relative offsets, topics/setting in metadata - assert entries[0] == { - "start": "00:00:00", - "speaker": "Jack", - "text": "Board meeting notes", - "source": "import", - } - assert entries[1] == { - "start": "00:00:30", - "speaker": "Ramon", - "text": "Action items", - "source": "import", - } - # Topics/setting extracted to metadata (not written as entry) - assert len(entries) == 2 - assert metadata["topics"] == "board meeting, action items" - assert metadata["setting"] == "workplace" - - # Verify .pdf auto-detected as text import (stream = import.text) - stream_json = day_dir / "import.text" / "163000_5" / "stream.json" - assert stream_json.exists() - stream_data = json.loads(stream_json.read_text()) - assert stream_data["stream"] == "import.text" - - # Verify segments.json written - segments_json = tmp_path / "imports" / "20251205_163000" / "segments.json" - assert segments_json.exists() + assert result is not None + assert result["source_type"] == "document" + assert output["importer"] == "document" + assert output["entries_written"] == 0 + assert any("corrupt.pdf" in error for error in output["errors"]) + assert not (tmp_path / "chronicle" / "20251205" / "import.text").exists() def test_write_segment(tmp_path): diff --git a/tests/test_pdf_lazy_imports.py b/tests/test_pdf_lazy_imports.py index eedf937a2..b87e02275 100644 --- a/tests/test_pdf_lazy_imports.py +++ b/tests/test_pdf_lazy_imports.py @@ -96,16 +96,3 @@ def test_document_importer_process_pdf_missing_extra(monkeypatch, tmp_path): DocumentImporter().process(pdf, tmp_path) assert "pip install 'solstone[pdf]'" in str(exc.value) - - -def test_read_transcript_pdf_missing_extra(monkeypatch, tmp_path): - _force_missing_pdf(monkeypatch) - from solstone.think.importers.text import _read_transcript - - pdf = tmp_path / "x.pdf" - pdf.write_bytes(b"") - - with pytest.raises(MissingExtraError) as exc: - _read_transcript(str(pdf)) - - assert "pip install 'solstone[pdf]'" in str(exc.value) -- 2.54.0