diff --git a/.agents/phases/todo/122_image_documents/00_phase.md b/.agents/phases/complete/122_image_documents/00_phase.md similarity index 100% rename from .agents/phases/todo/122_image_documents/00_phase.md rename to .agents/phases/complete/122_image_documents/00_phase.md diff --git a/.agents/phases/todo/122_image_documents/01_image_toggle.md b/.agents/phases/complete/122_image_documents/01_image_toggle.md similarity index 100% rename from .agents/phases/todo/122_image_documents/01_image_toggle.md rename to .agents/phases/complete/122_image_documents/01_image_toggle.md diff --git a/.agents/phases/todo/122_image_documents/02_image_ingest.md b/.agents/phases/complete/122_image_documents/02_image_ingest.md similarity index 100% rename from .agents/phases/todo/122_image_documents/02_image_ingest.md rename to .agents/phases/complete/122_image_documents/02_image_ingest.md diff --git a/.agents/phases/todo/122_image_documents/03_image_description.md b/.agents/phases/complete/122_image_documents/03_image_description.md similarity index 100% rename from .agents/phases/todo/122_image_documents/03_image_description.md rename to .agents/phases/complete/122_image_documents/03_image_description.md diff --git a/.agents/phases/todo/122_image_documents/04_serve_and_display.md b/.agents/phases/complete/122_image_documents/04_serve_and_display.md similarity index 100% rename from .agents/phases/todo/122_image_documents/04_serve_and_display.md rename to .agents/phases/complete/122_image_documents/04_serve_and_display.md diff --git a/.agents/phases/todo/122_image_documents/05_rag_display.md b/.agents/phases/complete/122_image_documents/05_rag_display.md similarity index 100% rename from .agents/phases/todo/122_image_documents/05_rag_display.md rename to .agents/phases/complete/122_image_documents/05_rag_display.md diff --git a/.agents/phases/todo/122_image_documents/06_image_tests.md b/.agents/phases/complete/122_image_documents/06_image_tests.md similarity index 100% rename from .agents/phases/todo/122_image_documents/06_image_tests.md rename to .agents/phases/complete/122_image_documents/06_image_tests.md diff --git a/.agents/reports/122_image_documents/122_image_documents__00_phase.a1.err b/.agents/reports/122_image_documents/122_image_documents__00_phase.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/122_image_documents/122_image_documents__00_phase.a1.md b/.agents/reports/122_image_documents/122_image_documents__00_phase.a1.md new file mode 100644 index 0000000..eeb6fdb --- /dev/null +++ b/.agents/reports/122_image_documents/122_image_documents__00_phase.a1.md @@ -0,0 +1,19 @@ +**Phase 122 (image documents) — final verification pass: all green. No code changes were needed; defects found: none.** + +**Verified (implementation already complete in working tree, reviewed end-to-end):** +- Toggle (`BOR_IMAGES`/`BOR_IMAGE_EXTENSIONS`/`BOR_IMAGE_DIR`, off by default) + `GET /api/config` `images` flag +- Ingest: bytes digest, `image_dir` persistent copy, `content = summary = vision description` (chat-model call; only text embedded), fail-soft skip + `images_failed` counter +- Serve/display: `/api/documents/{id}/image` route (404 matrix), viewer `` + description, Sources 48px lazy thumbnails, chat inline source figure (alt = summary), agent `read` marker +- Prune guard: images-off syncs never prune `is_image` docs + +**Test / lint / coverage (exact commands & outcomes):** +- `uv run pytest` → exit 0 (green; note: pytest 9.1.1 `-q` omits the final count line in output — exit code authoritative) +- `uv run pytest --cov=app --cov-report=term-missing` → **2715 passed, exit 0, TOTAL 99%** (>90% gate) +- `uv run ruff check . && uv run pyright` → "All checks passed!" / "0 errors, 0 warnings, 0 informations" +- `uv run pytest tests/e2e/test_image_documents.py -v --no-cov` → **4 passed, exit 0** (isolation) + +**Completion criteria:** (1) images=true → described/embedded/displayed docs: ✅ (E2E + integration) · (2) images=false byte-identical + image docs survive sync: ✅ (E2E negative app + unit/integration) · (3) viewer + chat rendering with alt text; failed description skips + logs, sync completes: ✅ · (4) test/lint/coverage gates: ✅ · (5) commit + phase move: deferred to harness per this pass's rules (working tree left uncommitted). + +**Notable deviation (pre-existing, documented in code):** image route uses `require_user` (phase-79 posture, same gate as the document content endpoint) rather than the phase text's "public" parenthetical — matches the endpoint it mirrors. + +**Next pending phase:** `123_chat_image_questions`. diff --git a/.agents/reports/122_image_documents/122_image_documents__00_phase.a1.validate b/.agents/reports/122_image_documents/122_image_documents__00_phase.a1.validate new file mode 100644 index 0000000..657c60d --- /dev/null +++ b/.agents/reports/122_image_documents/122_image_documents__00_phase.a1.validate @@ -0,0 +1,106 @@ +........................................................................ [ 2%] +........................................................................ [ 5%] +........................................................................ [ 7%] +........................................................................ [ 10%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 18%] +........................................................................ [ 21%] +........................................................................ [ 23%] +........................................................................ [ 26%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 34%] +........................................................................ [ 37%] +........................................................................ [ 39%] +........................................................................ [ 42%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 50%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 63%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 71%] +........................................................................ [ 74%] +........................................................................ [ 76%] +........................................................................ [ 79%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 87%] +........................................................................ [ 90%] +........................................................................ [ 92%] +........................................................................ [ 95%] +........................................................................ [ 98%] +................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 226 1 99% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 179 1 99% +app/api/git_sources.py 241 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 139 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 247 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 66 0 100% +app/models.py 131 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 349 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 206 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 38 0 100% +app/rag/importer.py 316 16 95% +app/rag/llm.py 244 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 102 0 100% +app/rag/retriever.py 248 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 46 0 100% +app/schemas.py 373 0 100% +-------------------------------------------------- +TOTAL 4514 29 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/122_image_documents/122_image_documents__01_image_toggle.a1.err b/.agents/reports/122_image_documents/122_image_documents__01_image_toggle.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/122_image_documents/122_image_documents__01_image_toggle.a1.md b/.agents/reports/122_image_documents/122_image_documents__01_image_toggle.a1.md new file mode 100644 index 0000000..43a9de8 --- /dev/null +++ b/.agents/reports/122_image_documents/122_image_documents__01_image_toggle.a1.md @@ -0,0 +1,19 @@ +All green. The untracked `.agents/reports/` dir is a harness runtime artifact — left untouched. + +## Task 01 complete — image toggle (`BOR_IMAGES` + extensions + dir, off by default) + +**Implemented** +- `app/config.py`: `images: bool = False`, `image_extensions: str = "png,jpg,jpeg,webp,gif,bmp"`, `image_dir: str = "~/bor-sources/images"` (house docstrings, LOCKED A3 default); `image_extension_set` property (dotted lowercased frozenset, separate from `import_extension_set`); fail-loud validator for empty/malformed `image_extensions` (the `import_extensions` precedent) +- `.env.example`: three entries + comment block (off by default, vision-model dependency note) +- `app/api/config.py`: `GET /api/config` gains `"images": settings.images` (six-key contract; docstring notes phase-123 composer consumption) +- Tests: new `tests/unit/test_image_documents.py` (7 tests: defaults, env overrides, case/trim parse, validators, set separation); `/api/config` pins updated to six keys in `test_api.py` (+ new `images=True` flag-tracking test), `test_save_as_doc_button.py`, `test_ui_settings_api.py`, and 2 E2E suites; conftest leak-pins for the 3 vars (house pattern) + +**Verification** +- `uv run pytest` → 2658 passed +- `uv run pytest --cov=app --cov-report=term-missing` → TOTAL **99%** (>90%); touched modules 100% +- `uv run ruff check . && uv run pyright` → clean (0 errors) +- E2E sanity (isolated): `test_configurable_brand.py` 5 passed, `test_ui_customization.py` 4 passed + +**Decisions**: added the `image_extensions` validator (not explicit in the task, but the design cites the `import_extensions` precedent and the house fail-loud rule); `images` placed after `docs_repo_configured` in the response dict. No live-infra touched. + +**Next pending task**: `.agents/phases/todo/122_image_documents/02_image_ingest.md` diff --git a/.agents/reports/122_image_documents/122_image_documents__01_image_toggle.a1.validate b/.agents/reports/122_image_documents/122_image_documents__01_image_toggle.a1.validate new file mode 100644 index 0000000..f8843d6 --- /dev/null +++ b/.agents/reports/122_image_documents/122_image_documents__01_image_toggle.a1.validate @@ -0,0 +1,105 @@ +........................................................................ [ 2%] +........................................................................ [ 5%] +........................................................................ [ 8%] +........................................................................ [ 10%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 18%] +........................................................................ [ 21%] +........................................................................ [ 24%] +........................................................................ [ 27%] +........................................................................ [ 29%] +........................................................................ [ 32%] +........................................................................ [ 35%] +........................................................................ [ 37%] +........................................................................ [ 40%] +........................................................................ [ 43%] +........................................................................ [ 46%] +........................................................................ [ 48%] +........................................................................ [ 51%] +........................................................................ [ 54%] +........................................................................ [ 56%] +........................................................................ [ 59%] +........................................................................ [ 62%] +........................................................................ [ 65%] +........................................................................ [ 67%] +........................................................................ [ 70%] +........................................................................ [ 73%] +........................................................................ [ 75%] +........................................................................ [ 78%] +........................................................................ [ 81%] +........................................................................ [ 83%] +........................................................................ [ 86%] +........................................................................ [ 89%] +........................................................................ [ 92%] +........................................................................ [ 94%] +........................................................................ [ 97%] +.................................................................. [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 226 1 99% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 156 1 99% +app/api/git_sources.py 241 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 139 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 247 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 66 0 100% +app/models.py 129 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 347 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 206 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 38 0 100% +app/rag/importer.py 219 3 99% +app/rag/llm.py 244 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 102 0 100% +app/rag/retriever.py 242 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 344 0 100% +-------------------------------------------------- +TOTAL 4333 16 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/122_image_documents/122_image_documents__02_image_ingest.a1.err b/.agents/reports/122_image_documents/122_image_documents__02_image_ingest.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/122_image_documents/122_image_documents__02_image_ingest.a1.md b/.agents/reports/122_image_documents/122_image_documents__02_image_ingest.a1.md new file mode 100644 index 0000000..96091ad --- /dev/null +++ b/.agents/reports/122_image_documents/122_image_documents__02_image_ingest.a1.md @@ -0,0 +1,17 @@ +Everything is complete and green. The diff is coherent and matches the task spec exactly. + +## Report — Task 02 (image ingest) complete + +**Implemented** +- `app/models.py`: `Document.is_image` (bool, server-default false) + `Document.image_path` (Text NULL), house docstrings +- `alembic/versions/0022_documents_image.py`: both columns + tested downgrade (A13); applied to dev DB +- `app/rag/importer.py`: walk admits `image_extension_set` only while `settings.images` (separate set, empty default = byte-identical off); `_index_file` image branch BEFORE `read_text` → `_index_image_file` (bytes sha256, persistent copy `image_dir/.` written only after successful description, stale-copy delete on change, `is_image`/`image_path` set, description = content, normal chunk/`_store_summary` pipeline); single commented seam `_describe_or_skip` for task 03; `_prune` guard (off → `is_image` docs survive; on → pruned + copy deleted); `ImportSummary.images_failed` + slot in the PLAN §9 log line +- Tests: unit section in `tests/unit/test_image_documents.py` (walk filter, binary branch incl. `read_text`-never-called, fail-soft skip + log line, unchanged/backfill, stale-copy delete, prune guard); `tests/integration/test_migration_0022.py` (upgrade/downgrade/round-trip, backfill, ORM); `tests/integration/test_docs_api.py` `import_sources` e2e (on/off/prune) + +**Results** +- `uv run pytest --cov=app --cov-report=term-missing` → 2672 passed, TOTAL 99% (>90% gate) +- `uv run ruff check .` → clean; `uv run pyright` → 0 errors + +**Decisions**: task-02 seam `_describe_or_skip` is a placeholder returning `None` (task 03's `describe_image` fills its body — per the task file's strict reading, since task 03 isn't merged); log-line pin and a walk-stub signature in `tests/unit/test_importer.py` updated for the additive changes. Note: source *removal* (`DELETE /api/git-sources/{id}`) doesn't yet delete image copies — outside this task's list; flag for a later task. + +**Next pending task**: `03_image_description.md` diff --git a/.agents/reports/122_image_documents/122_image_documents__02_image_ingest.a1.validate b/.agents/reports/122_image_documents/122_image_documents__02_image_ingest.a1.validate new file mode 100644 index 0000000..2f0e3ce --- /dev/null +++ b/.agents/reports/122_image_documents/122_image_documents__02_image_ingest.a1.validate @@ -0,0 +1,106 @@ +........................................................................ [ 2%] +........................................................................ [ 5%] +........................................................................ [ 8%] +........................................................................ [ 10%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 18%] +........................................................................ [ 21%] +........................................................................ [ 24%] +........................................................................ [ 26%] +........................................................................ [ 29%] +........................................................................ [ 32%] +........................................................................ [ 35%] +........................................................................ [ 37%] +........................................................................ [ 40%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 48%] +........................................................................ [ 51%] +........................................................................ [ 53%] +........................................................................ [ 56%] +........................................................................ [ 59%] +........................................................................ [ 61%] +........................................................................ [ 64%] +........................................................................ [ 67%] +........................................................................ [ 70%] +........................................................................ [ 72%] +........................................................................ [ 75%] +........................................................................ [ 78%] +........................................................................ [ 80%] +........................................................................ [ 83%] +........................................................................ [ 86%] +........................................................................ [ 88%] +........................................................................ [ 91%] +........................................................................ [ 94%] +........................................................................ [ 97%] +........................................................................ [ 99%] +........ [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 226 1 99% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 156 1 99% +app/api/git_sources.py 241 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 139 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 247 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 66 0 100% +app/models.py 131 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 347 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 206 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 38 0 100% +app/rag/importer.py 313 17 95% +app/rag/llm.py 244 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 102 0 100% +app/rag/retriever.py 242 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 344 0 100% +-------------------------------------------------- +TOTAL 4429 30 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/122_image_documents/122_image_documents__03_image_description.a1.err b/.agents/reports/122_image_documents/122_image_documents__03_image_description.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/122_image_documents/122_image_documents__03_image_description.a1.md b/.agents/reports/122_image_documents/122_image_documents__03_image_description.a1.md new file mode 100644 index 0000000..2fefd00 --- /dev/null +++ b/.agents/reports/122_image_documents/122_image_documents__03_image_description.a1.md @@ -0,0 +1,22 @@ +Everything is green and coherent. Final state of the task verified against all completion criteria. + +## Report — Task 03 (image description) complete + +**Implemented** +- `describe_image` in `app/rag/summarizer.py` — ONE CHAT-model (vision) call (LOCKED A3), multimodal user message (`DESCRIBE_PROMPT` text part + `image_url` data-URL part), reply stripped + capped at `summary_max_chars`, `None` on client error/empty/non-2xx (logs the reason; the importer's warning names the doc) +- New constants: `IMAGE_DESCRIPTION_MODE` marker + `DESCRIBE_PROMPT` (E2E-mock key, task 06 wires the branch), `IMAGE_MIMES` map + fallback (task 04 reuses for `Content-Type`) +- Importer: `_describe_or_skip` seam filled (real `describe_image`, mime from `IMAGE_MIMES`); `_store_summary` image-aware — for `is_image` docs the description IS the summary (verbatim, no `lite` call/pointer), so `content == summary` and the `is_summary` chunk mirrors it; the phase-118 backfill reuses the same path +- Widened `Embedder`/`SummaryLLM`/`FakeEmbedder` chat typings to multimodal (no `LLMClient` change needed); E2E mock `_user`/`_context` made list-safe for multimodal messages (byte-identical for string content) +- Tests: 8 new unit + 1 new integration (mock-vision end-to-end, **no seam patch**); fixed task-02's backfill test for task-03 semantics (an image's remaining summary failure class is now the `is_summary` chunk's embed, not a failing `lite` call) + +**Results** +- `uv run pytest` → 2681 passed +- `uv run pytest --cov=app --cov-report=term-missing` → TOTAL **99%** (summarizer 100%, importer 95%) +- `uv run ruff check . && uv run pyright` → clean +- E2E spot-checks (smoke / agent-document-tools / oneshot-retry, in isolation) → all passed + +**Decisions** +- `describe_image` follows the existing one-shot `chat()` convention (house phase-96 empty-content retry policy) — no extra retry loop added +- Image `doc.summary` carries the description verbatim (no `Source:` pointer line) + +**Next pending task:** `04_serve_and_display.md` diff --git a/.agents/reports/122_image_documents/122_image_documents__03_image_description.a1.validate b/.agents/reports/122_image_documents/122_image_documents__03_image_description.a1.validate new file mode 100644 index 0000000..f12181b --- /dev/null +++ b/.agents/reports/122_image_documents/122_image_documents__03_image_description.a1.validate @@ -0,0 +1,106 @@ +........................................................................ [ 2%] +........................................................................ [ 5%] +........................................................................ [ 8%] +........................................................................ [ 10%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 18%] +........................................................................ [ 21%] +........................................................................ [ 24%] +........................................................................ [ 26%] +........................................................................ [ 29%] +........................................................................ [ 32%] +........................................................................ [ 34%] +........................................................................ [ 37%] +........................................................................ [ 40%] +........................................................................ [ 42%] +........................................................................ [ 45%] +........................................................................ [ 48%] +........................................................................ [ 51%] +........................................................................ [ 53%] +........................................................................ [ 56%] +........................................................................ [ 59%] +........................................................................ [ 61%] +........................................................................ [ 64%] +........................................................................ [ 67%] +........................................................................ [ 69%] +........................................................................ [ 72%] +........................................................................ [ 75%] +........................................................................ [ 77%] +........................................................................ [ 80%] +........................................................................ [ 83%] +........................................................................ [ 85%] +........................................................................ [ 88%] +........................................................................ [ 91%] +........................................................................ [ 93%] +........................................................................ [ 96%] +........................................................................ [ 99%] +................. [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 226 1 99% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 156 1 99% +app/api/git_sources.py 241 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 139 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 247 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 66 0 100% +app/models.py 131 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 347 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 206 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 38 0 100% +app/rag/importer.py 316 16 95% +app/rag/llm.py 244 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 102 0 100% +app/rag/retriever.py 242 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 46 0 100% +app/schemas.py 344 0 100% +-------------------------------------------------- +TOTAL 4454 29 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/122_image_documents/122_image_documents__04_serve_and_display.a1.err b/.agents/reports/122_image_documents/122_image_documents__04_serve_and_display.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/122_image_documents/122_image_documents__04_serve_and_display.a1.md b/.agents/reports/122_image_documents/122_image_documents__04_serve_and_display.a1.md new file mode 100644 index 0000000..f738fdd --- /dev/null +++ b/.agents/reports/122_image_documents/122_image_documents__04_serve_and_display.a1.md @@ -0,0 +1,18 @@ +All gates green. Final report: + +**Implemented (task 04 — serve + display):** +- `GET /api/documents/{id}/image` (app/api/docs.py): exact bytes via `FileResponse`, `Content-Type` from the shared `IMAGE_MIMES` map, `Cache-Control: private, max-age=3600`; 404 `document not found` for missing/malformed id, text docs, NULL `image_path`, lost file +- Content endpoint gains `is_image` (always present) + `image_url` (absent for text docs, never null); tree file nodes gain the same affordance, row-driven (text nodes byte-identical to pre-phase) +- Viewer (`document.js`, shared page+modal core): `` block from `image_url` (alt = summary), description stays in the plain-content slot, onerror → "Image unavailable" note; duplicate summary panel suppressed when summary === content +- Sources page: fixed 48px lazy thumbnail (object-fit cover) before the path link, document-glyph fallback on fetch failure; CSS in theme tokens + +**Tests / gates (all green):** +- `uv run pytest --cov=app` → 2702 passed, TOTAL **99%** (>90%) +- `uv run ruff check . && uv run pyright` → clean (0 errors) +- New: route battery (6 ext Content-Types, exact bytes), 404 matrix, auth pin, content/tree wire tests (integration) + schema wire + frontend source pins + builder image-node tests (unit); spot-checked E2E `test_smoke`/`test_summary_in_viewer` → pass + +**Notable decisions:** +- **Flagged deviation:** task says "PUBLIC" citing anonymous content — stale: phase 79 (owner, 2026-08-22) gated the content endpoint `require_user` (A10: only shared chats anonymous). The image route uses the same `require_user` gate ("like the document content endpoint"); anonymous → 401 +- Thumbnail lives in the Path cell (the phase-97 file table has no icon column); tree affordance is row-driven so surviving image docs keep thumbnails after toggle-off + +**Next pending task:** `.agents/phases/todo/122_image_documents/05_rag_display.md` diff --git a/.agents/reports/122_image_documents/122_image_documents__04_serve_and_display.a1.validate b/.agents/reports/122_image_documents/122_image_documents__04_serve_and_display.a1.validate new file mode 100644 index 0000000..4247f5d --- /dev/null +++ b/.agents/reports/122_image_documents/122_image_documents__04_serve_and_display.a1.validate @@ -0,0 +1,106 @@ +........................................................................ [ 2%] +........................................................................ [ 5%] +........................................................................ [ 7%] +........................................................................ [ 10%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 18%] +........................................................................ [ 21%] +........................................................................ [ 23%] +........................................................................ [ 26%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 34%] +........................................................................ [ 37%] +........................................................................ [ 39%] +........................................................................ [ 42%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 50%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 58%] +........................................................................ [ 61%] +........................................................................ [ 63%] +........................................................................ [ 66%] +........................................................................ [ 69%] +........................................................................ [ 71%] +........................................................................ [ 74%] +........................................................................ [ 77%] +........................................................................ [ 79%] +........................................................................ [ 82%] +........................................................................ [ 85%] +........................................................................ [ 87%] +........................................................................ [ 90%] +........................................................................ [ 93%] +........................................................................ [ 95%] +........................................................................ [ 98%] +...................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 226 1 99% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 179 1 99% +app/api/git_sources.py 241 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 139 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 247 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 66 0 100% +app/models.py 131 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 347 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 206 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 38 0 100% +app/rag/importer.py 316 16 95% +app/rag/llm.py 244 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 102 0 100% +app/rag/retriever.py 242 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 46 0 100% +app/schemas.py 366 0 100% +-------------------------------------------------- +TOTAL 4499 29 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/122_image_documents/122_image_documents__05_rag_display.a1.err b/.agents/reports/122_image_documents/122_image_documents__05_rag_display.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/122_image_documents/122_image_documents__05_rag_display.a1.md b/.agents/reports/122_image_documents/122_image_documents__05_rag_display.a1.md new file mode 100644 index 0000000..9e5eb4d --- /dev/null +++ b/.agents/reports/122_image_documents/122_image_documents__05_rag_display.a1.md @@ -0,0 +1,18 @@ +Everything is complete and green. Final report: + +**Task 05 (RAG display) — implemented** +- `app/schemas.py` — `SourceRef` gains optional `image_url` (bytes-route path); wrap-serializer drops it when `None` → text-doc frames byte-identical, never `null` +- `app/rag/retriever.py` — shared helper `source_ref_with_image(doc)` sets `image_url` iff `doc.is_image`; `app/api/chat.py` cited + related tiers both route through it (lockstep) +- `app/rag/agent.py` — `IMAGE_DOC_MARKER` constant; `read` of an image doc gets the marker line between the byte-identical header/date lines and the description (plain + truncation paths); text results byte-identical +- `frontend/assets/app.js` — `appendSourceImageFigure` + `fetchContentSummary`, gated on `s.image_url` in both `appendSources` and `appendRelated`: compact inline `` (additive — chip text/affordance kept), alt + visible caption = doc summary (fetched from the existing content endpoint — the frame carries no summary per assumption 5; title until it settles), img error removes the figure (collapses to plain chip, never a broken icon) +- `frontend/assets/styles.css` — `.source-image*` rules (96px cap, `object-fit: contain`, `--surface`, AA 8.6:1 caption, flat underline hover) +- Tests: 9 new unit tests in `tests/unit/test_image_documents.py` (helper present/absent + nested done-frame pin + pre-phase saved-ref parse, marker pins incl. truncation, house-style frontend contracts) + 2 integration tests in `test_chat_api.py` (image doc's ref alone carries `image_url`; text-only grounded turn → raw wire frame contains no `image_url` bytes, refs keep exactly the pre-122 key set) + +**Gates (exact):** +- `uv run pytest --cov=app --cov-report=term` → **2714 passed**, TOTAL **99%** (>90%) +- `uv run ruff check .` → **All checks passed** +- `uv run pyright` → **0 errors, 0 warnings** + +**Notable decisions:** summary caption fetched client-side (frame carries no summary — `image_url` is the only new frame field); the figure also renders in the related-docs row (same ref shape, same helper); E2E rendering scenario is task 06's scope per the task file. No live-infrastructure changes. + +**Next pending task:** `06_image_tests.md` (phase-122 test suite: unit consolidation + integration extensions + isolated E2E `test_image_documents.py`). diff --git a/.agents/reports/122_image_documents/122_image_documents__05_rag_display.a1.validate b/.agents/reports/122_image_documents/122_image_documents__05_rag_display.a1.validate new file mode 100644 index 0000000..7000c40 --- /dev/null +++ b/.agents/reports/122_image_documents/122_image_documents__05_rag_display.a1.validate @@ -0,0 +1,106 @@ +........................................................................ [ 2%] +........................................................................ [ 5%] +........................................................................ [ 7%] +........................................................................ [ 10%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 18%] +........................................................................ [ 21%] +........................................................................ [ 23%] +........................................................................ [ 26%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 34%] +........................................................................ [ 37%] +........................................................................ [ 39%] +........................................................................ [ 42%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 50%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 58%] +........................................................................ [ 61%] +........................................................................ [ 63%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 71%] +........................................................................ [ 74%] +........................................................................ [ 76%] +........................................................................ [ 79%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 87%] +........................................................................ [ 90%] +........................................................................ [ 92%] +........................................................................ [ 95%] +........................................................................ [ 98%] +.................................................. [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 226 1 99% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 179 1 99% +app/api/git_sources.py 241 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 139 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 247 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 66 0 100% +app/models.py 131 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 349 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 206 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 38 0 100% +app/rag/importer.py 316 16 95% +app/rag/llm.py 244 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 102 0 100% +app/rag/retriever.py 248 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 46 0 100% +app/schemas.py 373 0 100% +-------------------------------------------------- +TOTAL 4514 29 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/122_image_documents/122_image_documents__06_image_tests.a1.err b/.agents/reports/122_image_documents/122_image_documents__06_image_tests.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/122_image_documents/122_image_documents__06_image_tests.a1.md b/.agents/reports/122_image_documents/122_image_documents__06_image_tests.a1.md new file mode 100644 index 0000000..3ad2b52 --- /dev/null +++ b/.agents/reports/122_image_documents/122_image_documents__06_image_tests.a1.md @@ -0,0 +1,20 @@ +All checks complete. The phase 122 task 06 is done — all gates green. + +## Report + +**Implemented (task 06 — image tests):** +- `tests/e2e/test_image_documents.py` (new, isolated): module app with `BOR_IMAGES=true` (leak-guard env) + a second default-env app (`BOR_IMAGES=false`); real admin upload flow (single-PNG zip) → UI "Sync sources" → Sources thumbnail, document-viewer image+description, chat sources inline figure, and the default-off negative case (same upload → no image doc, "0 added · 0 unchanged") +- `tests/e2e/mock_llm.py`: `IMAGE_DESCRIPTION_MODE` branch + `IMAGE_DESCRIPTION_ANSWER` (deterministic, token-dense so the question grounds at cosine ≈0.31) +- `tests/integration/test_docs_api.py`: added the task's failing-vision integration pin (`images_failed == 1`, no row, text doc indexed, warning names the doc) +- **Defect fix (task 04 work):** `frontend/assets/sources.js` `renderLevel` built the `makeRow` row without `is_image`/`image_url`/`summary` — the Sources thumbnail never rendered; now passed through (text nodes stay undefined → byte-identical bare link) + +**Test / lint / coverage:** +- `uv run pytest` → **2715 passed** (baseline 2714 + 1) +- `uv run pytest --cov=app --cov-report=term` → **TOTAL 99%** (>90% gate) +- `uv run pytest tests/e2e/test_image_documents.py -v --no-cov` → **4 passed** (ran 3× incl. consecutive stability; DB up) +- `uv run ruff check .` → clean; `uv run pyright` → 0 errors +- Cross-check: `test_chat_rag.py` + `test_document_viewer.py` still pass (11) — mock change is additive; restored an unrelated screenshot regenerated by the cross-check run + +**Decisions:** chat test uses the phase-119 A1 scripted-read convention (zero-read turns chip nothing — the image doc must be read to earn chip+figure); both E2E apps pin `BOR_IMAGES` explicitly so an operator `.env` can't leak the toggle either direction. + +**Next pending task:** none in `122_image_documents` — phase 122 complete (harness moves task 06 + commits). Next phase: `123_chat_image_submission` (todo). diff --git a/.agents/reports/122_image_documents/122_image_documents__06_image_tests.a1.validate b/.agents/reports/122_image_documents/122_image_documents__06_image_tests.a1.validate new file mode 100644 index 0000000..657c60d --- /dev/null +++ b/.agents/reports/122_image_documents/122_image_documents__06_image_tests.a1.validate @@ -0,0 +1,106 @@ +........................................................................ [ 2%] +........................................................................ [ 5%] +........................................................................ [ 7%] +........................................................................ [ 10%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 18%] +........................................................................ [ 21%] +........................................................................ [ 23%] +........................................................................ [ 26%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 34%] +........................................................................ [ 37%] +........................................................................ [ 39%] +........................................................................ [ 42%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 50%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 63%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 71%] +........................................................................ [ 74%] +........................................................................ [ 76%] +........................................................................ [ 79%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 87%] +........................................................................ [ 90%] +........................................................................ [ 92%] +........................................................................ [ 95%] +........................................................................ [ 98%] +................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 226 1 99% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 179 1 99% +app/api/git_sources.py 241 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 139 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 247 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 66 0 100% +app/models.py 131 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 349 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 206 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 38 0 100% +app/rag/importer.py 316 16 95% +app/rag/llm.py 244 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 102 0 100% +app/rag/retriever.py 248 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 46 0 100% +app/schemas.py 373 0 100% +-------------------------------------------------- +TOTAL 4514 29 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.env.example b/.env.example index a01af94..ad7d256 100644 --- a/.env.example +++ b/.env.example @@ -112,6 +112,20 @@ BOR_IMPORT_EXTENSIONS=md,markdown,txt,yaml,yml,json,py,container,network,volume, # BOR_UPLOAD_MAX_MB=512 # caps BOTH the compressed upload and the total # extracted bytes (zip-bomb guard); must be > 0 +# --- Image documents (phase 122) --- +# Master switch for indexing standalone image files (direct uploads, +# uploaded archives, git/local source walks) as first-class documents. +# OFF by default — enable only when BOR_LLM_CHAT_MODEL supports vision: +# the chat model generates the image's description, and that +# description is the ONLY part of the image that gets indexed (the +# embedding model never sees pixels). While off, image files are +# ignored by the import walks and a sync never prunes existing image +# documents. +# BOR_IMAGES=0 # 1 = index standalone images +# BOR_IMAGE_EXTENSIONS=png,jpg,jpeg,webp,gif,bmp # comma-separated, case-insensitive +# BOR_IMAGE_DIR=~/bor-sources/images # persistent home for the served image bytes + # (uploads are replaced, checkouts re-cloned) + # --- Docs push (phase 59: save a chat answer as documentation) --- # The git repo chat answers can be committed to — any remote (URL or # local path). While empty, the "Save as doc" action is hidden and the diff --git a/alembic/versions/0022_documents_image.py b/alembic/versions/0022_documents_image.py new file mode 100644 index 0000000..e902af3 --- /dev/null +++ b/alembic/versions/0022_documents_image.py @@ -0,0 +1,54 @@ +"""documents image columns: is_image + image_path (phase 122, task 02) + +Revision ID: 0022 +Revises: 0021 +Create Date: 2026-09-24 + +Phase 122 (standalone images become first-class documents — task 02, +storage only): + +* ``documents.is_image`` — BOOLEAN NOT NULL, server default + ``false``: True iff the doc is a standalone image (LOCKED A3) whose + ``content``/``summary`` is the CHAT model's vision description — the + ONLY embedded text (the embedding model never sees pixels). The + server default makes EVERY pre-phase-122 row a text doc without a + backfill. +* ``documents.image_path`` — TEXT NULLABLE: the absolute path of the + image's persistent copy in ``settings.image_dir`` (``.`` + — the copy must outlive the source file: uploads are replaced on + every upload, git checkouts are re-cloned). NULL for text docs. + +One additive, fully reversible migration (A13); no other schema +change. The walk filter, the binary index path, and the prune guard +are importer code (task 02) — this revision only carries the columns. +""" +from __future__ import annotations + +import sqlalchemy as sa + +from alembic import op + +revision = "0022" +down_revision = "0021" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column( + "documents", + sa.Column( + "is_image", + sa.Boolean(), + server_default=sa.text("false"), + nullable=False, + ), + ) + op.add_column("documents", sa.Column("image_path", sa.Text(), nullable=True)) + + +def downgrade() -> None: + # Both columns are the only 0022 artefacts — dropping them leaves + # 0021's schema byte-identical (A13, fully reversible). + op.drop_column("documents", "image_path") + op.drop_column("documents", "is_image") diff --git a/app/api/chat.py b/app/api/chat.py index e7c31f3..80dc74c 100644 --- a/app/api/chat.py +++ b/app/api/chat.py @@ -224,6 +224,7 @@ from app.rag.retriever import ( retrieve, select_related, select_suggested, + source_ref_with_image, weak_hit_titles, ) from app.rag.scaffolding import ScaffoldingFilter # phase 71: the streaming filter @@ -984,14 +985,16 @@ async def chat( # since phase 119). The UI renders the row as the # de-emphasized related-docs row, never a citation chip; # old clients ignore the field. + # Phase 122 (task 05): both tiers build their refs + # through the shared helper — a ref for an image doc + # carries the optional ``image_url`` (the bytes route), + # a text doc's stays byte-identical to pre-phase (the + # key is omitted, never null). cited_refs: list[SourceRef] = [] if not plan.deflected: - cited_refs = [ - SourceRef(source=d.source, path=d.path, title=d.title) - for d in cited_docs - ] + cited_refs = [source_ref_with_image(d) for d in cited_docs] related_refs = [ - SourceRef(source=d.source, path=d.path, title=d.title) + source_ref_with_image(d) for d in plan.related_docs if (d.source, d.path) not in cited_seen ] diff --git a/app/api/config.py b/app/api/config.py index bb06fb4..ee064ff 100644 --- a/app/api/config.py +++ b/app/api/config.py @@ -1,6 +1,7 @@ """Public app metadata (display name + version) for the frontend brand -layer, the phase-59 docs-push flag (the "Save as doc" gating), and the -phase-62 UI customization strings (composer placeholder, footer line). +layer, the phase-59 docs-push flag (the "Save as doc" gating), the +phase-122 image flag (UI affordance gating), and the phase-62 UI +customization strings (composer placeholder, footer line). Phase 91 (task 01): the three UI strings are now the EFFECTIVE values — the ``ui_settings`` row (admin Theme tab) over the env values (B1: DB @@ -29,17 +30,21 @@ router = APIRouter(tags=["config"]) @router.get("/config") def app_config(settings: Settings = Depends(get_settings)) -> dict[str, str | bool]: # noqa: B008 """Public app metadata for the frontend brand layer (phase 39) + - the phase-59 ``docs_repo_configured`` flag + the phase-62 UI - customization keys (``input_placeholder``, ``footer_text``) — all - display strings, the SAME boot fetch (no new network surface) and - the same public posture as ``app_name`` (no secrets). Phase 91: + the phase-59 ``docs_repo_configured`` flag + the phase-122 + ``images`` flag + the phase-62 UI customization keys + (``input_placeholder``, ``footer_text``) — all display strings, + the SAME boot fetch (no new network surface) and the same public + posture as ``app_name`` (no secrets). Phase 91: ``app_name`` / ``input_placeholder`` / ``footer_text`` are the EFFECTIVE values (the admin Theme tab's ``ui_settings`` row over the env values — DB-over-env, B1); the frontend brand layer treats an empty string as "keep the template default" (the unset => byte-identical contract). Phase 91 (task 03): the retired - CSS-file theming's ``theme`` key is gone — the five keys are the - entire response.""" + CSS-file theming's ``theme`` key is gone. Phase 122 (task 01): + ``images`` mirrors ``settings.images`` (the ``BOR_IMAGES`` master + switch) — consumed by the chat composer (phase 123) to show/hide + the image-attach control, optionally by the Sources page (an + "images off" hint). The six keys are the entire response.""" db = SessionLocal() try: effective = theming.effective_settings(db, settings) @@ -49,6 +54,7 @@ def app_config(settings: Settings = Depends(get_settings)) -> dict[str, str | bo "app_name": effective["app_name"], "version": settings.app_version, "docs_repo_configured": settings.docs_configured, + "images": settings.images, "input_placeholder": effective["input_placeholder"], "footer_text": effective["footer_text"], } diff --git a/app/api/docs.py b/app/api/docs.py index 3ee3c98..6979aa7 100644 --- a/app/api/docs.py +++ b/app/api/docs.py @@ -23,6 +23,18 @@ GET /api/docs/tree — the admin's full recursive KB tree in one fetch walks, with the file metadata the RAG view's rows and stat cards need (the view drills client-side; ``GET /api/docs`` is untouched). +GET /api/documents/{doc_id}/image — the phase-122 (task 04) image +BYTES route: the persistent copy behind an ``is_image`` document's +``image_path``, served with the extension's ``Content-Type`` (the +``app.rag.summarizer.IMAGE_MIMES`` map — one map, one truth) and a +``Cache-Control: private, max-age=3600`` header (the bytes are +content-hashed — long enough, bustable by re-upload). User-gated like +the content endpoint (phase 79 — the ONLY anonymous surface is the +shared chats): a missing doc, a non-image doc, a doc whose +``image_path`` is NULL, or a row whose copy was lost all map to 404 +``document not found`` (the router's unknown-document shape — +traversal/UUID-guessing has no filesystem surface to hit). + PATCH /api/folders/summary — the admin folder-description editor (phase 97, task 03): update / create / clear a stored ``folder_summaries`` row, marking every non-empty save @@ -34,11 +46,13 @@ contrast with the phase-57 ``is_summary`` re-embed above. """ from __future__ import annotations +import uuid from collections.abc import Mapping, Sequence from datetime import UTC, datetime from pathlib import Path from fastapi import APIRouter, Depends, HTTPException +from fastapi.responses import FileResponse from sqlalchemy import func, select from sqlalchemy.orm import Session @@ -52,6 +66,7 @@ from app.rag.doc_dates import normalize_doc_date from app.rag.folder_summaries import MIN_DOCS_PER_FOLDER, folder_of from app.rag.importer import match_extension from app.rag.llm import EmbeddingError, LLMClient +from app.rag.summarizer import IMAGE_FALLBACK_MIME, IMAGE_MIMES from app.schemas import ( DateResult, DateUpdate, @@ -171,6 +186,11 @@ def get_document_content( if row is None: raise HTTPException(status_code=404, detail="document not found") doc, chunks = row + # Phase 122 (task 04): the image affordance — ``is_image`` is ALWAYS + # present on the wire (text docs: false — the one new key); for an + # image doc, ``image_url`` is the bytes route's path (absent for + # text docs, and for an image row whose copy path is NULL — never + # null, the ``DocContent`` omission rule). return DocContent( source=doc.source, path=doc.path, @@ -181,6 +201,62 @@ def get_document_content( content=doc.content, indexed_at=doc.indexed_at.isoformat(), chunks=chunks, + is_image=doc.is_image, + image_url=( + f"/api/documents/{doc.id}/image" if doc.is_image and doc.image_path else None + ), + ) + + +@router.get("/documents/{doc_id}/image", response_class=FileResponse) +def get_document_image( + doc_id: str, + db: Session = Depends(get_db), # noqa: B008 + _user: None = Depends(require_user), # noqa: B008 # phase 79 posture (see docstring) +) -> FileResponse: + """The phase-122 (task 04) image BYTES route — the persistent copy + behind an ``is_image`` document's ``image_path``. + + Same auth posture as the document content endpoint (phase 79 — + ``require_user``: admin OR live token holder; the ONLY anonymous + surface is the shared chats — the image is part of a document's + content, so it travels under the same gate): anonymous callers get + 401 ``authentication required`` before any row is read. + + 404 ``document not found`` (the router's unknown-document shape) in + every non-servable case — a missing id (an unparseable string maps + here too, not to a 422 — a guessed id is an unknown document), a + text doc, an image doc whose ``image_path`` is NULL, or a row whose + copy was lost on disk (defensive — the row exists, the bytes + don't). There is no path parameter to a filesystem value: the path + comes from the ROW (the importer's ``image_dir`` copy), so there is + no traversal surface. + + Servable rows stream the exact bytes with the extension's + ``Content-Type`` (the ``IMAGE_MIMES`` map — one map, one truth with + the describe call's data-URL mime; an unexpected extension takes + ``application/octet-stream``) and ``Cache-Control: private, + max-age=3600`` (the bytes are content-hashed — long enough to be + useful, bustable by re-upload). + """ + try: + uid = uuid.UUID(doc_id) + except ValueError: + raise HTTPException(status_code=404, detail="document not found") from None + doc = db.scalar(select(Document).where(Document.id == uid)) + if doc is None or not doc.is_image or not doc.image_path: + raise HTTPException(status_code=404, detail="document not found") + image_file = Path(doc.image_path) + if not image_file.is_file(): + # Defensive: the row exists but the copy was lost (the owner + # cleaned the image dir, the disk was wiped) — the viewer's + # onerror fallback renders the "image unavailable" note. + raise HTTPException(status_code=404, detail="document not found") + media_type = IMAGE_MIMES.get(image_file.suffix.lower(), IMAGE_FALLBACK_MIME) + return FileResponse( + image_file, + media_type=media_type, + headers={"Cache-Control": "private, max-age=3600"}, ) @@ -446,6 +522,13 @@ def _folder_counts( return folders, counts +#: One image-docs map value (phase 122, task 04): +#: ``(doc_id, summary)`` — the id the bytes route's URL is built from +#: and the summary the RAG view's thumbnail uses as ``alt`` (the vision +#: description; ``None`` = the fail-soft backfill corner). +ImageDocInfo = tuple[str, str] + + def _level_children( source: str, folder: str, @@ -453,6 +536,7 @@ def _level_children( counts: dict[str, int], rows: Sequence[TreeFileRow], summaries: Mapping[tuple[str, str], str], + images: Mapping[tuple[str, str], ImageDocInfo] | None = None, ) -> list[KbTreeFolder | KbTreeFile]: """One level's children (pure): subfolders in path order, then the direct files in input (catalog) order. @@ -477,10 +561,19 @@ def _level_children( = the subtree's MAX document ``created_at``: the max over this folder's direct files' dates and its subfolder children's (already recursive) ``updated_at`` values, via :func:`_subtree_max`. + + Since phase 122 (task 04), a file node whose ``(source, path)`` is + in *images* carries the thumbnail affordance (``is_image`` + + ``image_url`` built from the mapped doc id + the mapped ``summary`` + — see :class:`app.schemas.KbTreeFile`); every other file node is + the pre-phase shape (the omission rule keeps its wire shape + byte-identical). """ children: list[KbTreeFolder | KbTreeFile] = [] for sub in sorted(g for g in folders if folder_of(g) == folder): - sub_children = _level_children(source, sub, folders, counts, rows, summaries) + sub_children = _level_children( + source, sub, folders, counts, rows, summaries, images + ) children.append( KbTreeFolder( path=sub, @@ -494,15 +587,34 @@ def _level_children( ) for path, title, chunks, indexed_at, created_at in rows: if folder_of(path) == folder: - children.append( - KbTreeFile( - path=path, - title=title, - chunks=chunks, - created_at=created_at, - indexed_at=indexed_at, + image_info = images.get((source, path)) if images else None + if image_info is not None: + # Phase 122 (task 04): the image-docs node — the RAG + # view's Path cell renders the 48px thumbnail from the + # bytes route's URL with ``alt = summary``. + doc_id, doc_summary = image_info + children.append( + KbTreeFile( + path=path, + title=title, + chunks=chunks, + created_at=created_at, + indexed_at=indexed_at, + is_image=True, + image_url=f"/api/documents/{doc_id}/image", + summary=doc_summary, + ) + ) + else: + children.append( + KbTreeFile( + path=path, + title=title, + chunks=chunks, + created_at=created_at, + indexed_at=indexed_at, + ) ) - ) return children @@ -530,6 +642,7 @@ def build_kb_tree( names: Sequence[str], doc_rows: Sequence[TreeDocRow], summaries: Mapping[tuple[str, str], str], + images: Mapping[tuple[str, str], ImageDocInfo] | None = None, ) -> list[KbTreeSource]: """The pure tree builder behind ``GET /api/docs/tree`` (phase 97, task 02) — module-level and DB-free so unit tests drive it @@ -543,7 +656,13 @@ def build_kb_tree( ``{(source, folder_path): summary}`` over the stored ``folder_summaries`` rows (``folder_path = ""`` = the source root; rows for sources the tree does not list are simply never - referenced). + referenced). *images* (phase 122, task 04) — + ``{(source, path): (doc_id, summary)}`` over the stored image docs + (``is_image`` rows with a servable ``image_path`` — the endpoint + composes the bounded select); the default ``None``/empty map keeps + EVERY file node the pre-phase shape (byte-identical wire — a + pre-phase KB has no image rows, so the endpoint's own map is empty + for it). Shape, per the phase-97 ``00_phase.md`` "The tree endpoint": @@ -614,10 +733,10 @@ def build_kb_tree( if name in listed: # defensive: list_source_names dedupes continue listed.add(name) - tree.append(_source_node(name, by_source.get(name, ()), summaries)) + tree.append(_source_node(name, by_source.get(name, ()), summaries, images)) for source in sorted(by_source): if source not in listed: - tree.append(_source_node(source, by_source[source], summaries)) + tree.append(_source_node(source, by_source[source], summaries, images)) return tree @@ -625,6 +744,7 @@ def _source_node( source: str, rows: Sequence[TreeFileRow], summaries: Mapping[tuple[str, str], str], + images: Mapping[tuple[str, str], ImageDocInfo] | None = None, ) -> KbTreeSource: """One source node (pure): whole-source count + the source-root summary + the root level's children (direct subfolders + direct @@ -646,7 +766,7 @@ def _source_node( 0-document source (no children, no dates). """ folders, counts = _folder_counts(rows) - children = _level_children(source, "", folders, counts, rows, summaries) + children = _level_children(source, "", folders, counts, rows, summaries, images) return KbTreeSource( name=source, documents=len(rows), @@ -677,9 +797,12 @@ def list_kb_tree( excluded — the tree has no document ids) + ALL stored ``folder_summaries`` rows (a bounded select — one row per existing folder at the ≥ 1-doc rule; rows for sources the tree - does not list are never referenced by the builder) — through the - pure :func:`build_kb_tree`. ``GET /api/docs`` itself is - untouched. + does not list are never referenced by the builder) + the phase-122 + (task 04) image-docs map (a second bounded select over the + ``is_image`` rows with a servable ``image_path`` — empty for every + pre-phase KB, so the response stays byte-identical to pre-phase) + — through the pure :func:`build_kb_tree`. ``GET /api/docs`` itself + is untouched. """ names = list_source_names(db) rows = db.execute( @@ -712,4 +835,22 @@ def list_kb_tree( select(FolderSummary.source, FolderSummary.folder_path, FolderSummary.summary) ).all() } - return KbTree(sources=build_kb_tree(names, doc_rows, summaries)) + # Phase 122 (task 04): the image-docs affordance map — a bounded + # select over the ``is_image`` rows only (a handful of rows at KB + # scale, never the whole catalog; the catalogue query above stays + # byte-identical). EMPTY for every pre-phase KB (no image rows), so + # the response stays byte-identical to pre-phase — the fields are + # row-driven, not toggle-driven (a surviving image doc keeps its + # thumbnail through a toggle-off sync, the prune guard's UX side). + images: dict[tuple[str, str], tuple[str, str]] = { + (source, path): (str(doc_id), summary) + for source, path, doc_id, summary in db.execute( + select( + Document.source, + Document.path, + Document.id, + Document.summary, + ).where(Document.is_image.is_(True), Document.image_path.is_not(None)) + ).all() + } + return KbTree(sources=build_kb_tree(names, doc_rows, summaries, images)) diff --git a/app/config.py b/app/config.py index 23f56ac..273e839 100644 --- a/app/config.py +++ b/app/config.py @@ -366,6 +366,36 @@ class Settings(BaseSettings): #: pattern). upload_max_mb: int = 512 + # --- Image documents (phase 122: standalone images as documents) --- + #: Master switch for image-document indexing (phase 122, + #: ``BOR_IMAGES``; ``0``/``false`` = off — the DEFAULT, LOCKED A3). + #: Enable only when ``llm_chat_model`` supports vision: image + #: descriptions are generated by the chat model, and the description + #: is the ONLY part of an image that gets indexed (the embedding + #: model never sees pixels). While off, the import walks ignore + #: image files and a sync never prunes existing ``is_image`` + #: documents (the phase-122 prune guard — the image is invisible to + #: an images-off walk, not a deleted file). + images: bool = False + #: Comma-separated, case-insensitive file extensions (no dot) treated + #: as standalone images when ``images`` is on (phase 122, + #: ``BOR_IMAGE_EXTENSIONS``). Stored as a raw CSV string (the + #: ``import_extensions`` house convention) and parsed on demand via + #: :py:meth:`image_extension_set`. A SEPARATE set from + #: ``import_extension_set`` — images are never user-added via + #: ``BOR_IMPORT_EXTENSIONS`` (the ``images`` toggle is the single + #: knob). The validator rejects an empty list and malformed tokens, + #: exactly like ``import_extensions`` (a typo would otherwise index + #: zero images silently). + image_extensions: str = "png,jpg,jpeg,webp,gif,bmp" + #: Where ingested image bytes are copied for serving (phase 122, + #: ``BOR_IMAGE_DIR``). Raw string — ``Path.expanduser()`` is applied + #: by the importer, not here (the ``sources_dir``/``upload_dir`` + #: convention). Deliberately separate from ``sources_dir`` (git + #: checkouts, re-cloned) and ``upload_dir`` (replaced on every + #: upload): the served copy must outlive the source file. + image_dir: str = "~/bor-sources/images" + # --- Docs push (phase 59: save a chat answer as documentation) --- #: The git repo a saved chat answer is committed to (phase 59, D3): #: **any** remote — a URL (``https://``, ``ssh://``, ``git@``) or a @@ -471,6 +501,25 @@ class Settings(BaseSettings): ) return v + @field_validator("image_extensions") + @classmethod + def _image_extensions_known(cls, v: str) -> str: + """Reject an empty list or malformed tokens loudly (the + ``import_extensions`` precedent, phase 122): a typo like + ``png,jpeb`` would otherwise index zero images silently.""" + exts = {part.strip().lstrip(".").lower() for part in v.split(",") if part.strip()} + if not exts: + raise ValueError("image_extensions must name at least one format") + malformed = sorted( + ext for ext in exts if re.fullmatch(r"[a-z0-9]{1,16}", ext) is None + ) + if malformed: + raise ValueError( + f"image_extensions contains malformed token(s): {', '.join(malformed)} — " + "each extension must be lowercase letters/digits only, 1-16 chars, no dot" + ) + return v + @field_validator("agent_max_rounds") @classmethod def _agent_max_rounds_non_negative(cls, v: int) -> int: @@ -647,6 +696,19 @@ class Settings(BaseSettings): if part.strip() ) + @property + def image_extension_set(self) -> frozenset[str]: + """Lowercased, dotted image-extension set (``.png``) for the + phase-122 walk filter — SEPARATE from + :py:attr:`import_extension_set` (images are never user-added via + ``BOR_IMPORT_EXTENSIONS``; the ``images`` toggle is the single + knob).""" + return frozenset( + f".{part.strip().lstrip('.').lower()}" + for part in self.image_extensions.split(",") + if part.strip() + ) + @property def git_source_list(self) -> list[str]: """Non-empty, stripped git URLs from :py:attr:`git_sources` (phase 28). diff --git a/app/models.py b/app/models.py index d86ba35..3a7f7d8 100644 --- a/app/models.py +++ b/app/models.py @@ -139,6 +139,29 @@ class Document(Base): #: failed, and until the phase-118 backfill stores one on the next #: sync. summary: Mapped[str | None] = mapped_column(Text, default=None) + #: True iff this document is a standalone image (phase 122, + #: LOCKED A3): ``content`` (and ``summary``) is the CHAT model's + #: vision description of the image — the ONLY embedded text (the + #: embedding model never sees pixels), and the image bytes + #: themselves live at :py:attr:`image_path` (served by the document + #: image route, task 04). ``False`` for every text document, + #: including all pre-phase-122 rows (the server default keeps them + #: valid without a backfill). An ``is_image`` doc is INVISIBLE to + #: an images-off walk, not a deleted file — the importer's prune + #: guard (the phase-122 derived decision) protects it while the + #: toggle is off. + is_image: Mapped[bool] = mapped_column( + Boolean, default=False, server_default=text("false"), nullable=False + ) + #: Absolute path of the image's PERSISTENT copy in + #: ``settings.image_dir`` (phase 122) — the importer copies each + #: ingested image there (``.``) because the source + #: file is disposable: uploads are replaced on every upload, git + #: checkouts are re-cloned, local dirs are user-edited. The copy is + #: written only when the doc is new or its hash changes, deleted on + #: a content change (the stale copy) and on prune. NULL for text + #: documents. + image_path: Mapped[str | None] = mapped_column(Text, default=None) chunks: Mapped[list[Chunk]] = relationship( back_populates="document", cascade="all, delete-orphan" diff --git a/app/rag/agent.py b/app/rag/agent.py index 12a1162..f6e6ff8 100644 --- a/app/rag/agent.py +++ b/app/rag/agent.py @@ -427,6 +427,18 @@ READ_TRUNCATION_NOTICE = ( "document." ) +#: The image-document marker (phase 122, task 05): the line prefixed to +#: the vision DESCRIPTION a ``read`` of a standalone-image document +#: returns — the model must reason about what it is reading (the text +#: below is a description GENERATED from the image, not the image's +#: own words). It sits on the result's THIRD line: the ``Document …`` +#: header and the phase-106 date line stay byte-identical (the E2E +#: mock's ``_READ_RESULT_PREFIX`` header contract), and a NON-image +#: doc's result carries no marker at all (byte-identical to pre-122). +IMAGE_DOC_MARKER = ( + "Image document — the text below is a description generated from the image:" +) + #: The no-source ``ls`` refusal with the teaching parenthetical #: appended (phase 72): used when a stripped scope has no ``/`` and #: matches no registered source (the incident's ``ls(path='.')``). The @@ -1270,6 +1282,12 @@ def _execute_tool( return _no_document_refusal(db, arg) holder.read_docs.append(doc) holder.tool_calls += 1 + # Phase 122 (task 05): an image document's content IS the vision + # description — the marker line (a third line between the + # byte-identical header/date lines and the text) tells the model + # what it is reading. A text doc's ``marker`` is "" — the result + # stays byte-identical to pre-122. + marker = f"{IMAGE_DOC_MARKER}\n" if doc.is_image else "" cap = settings.read_max_chars if len(doc.content) > cap: # Phase 95 (owner permission 2026-09-10, ``TODO.md`` L5): the @@ -1293,17 +1311,20 @@ def _execute_tool( return ( f"Document {doc.source}/{doc.path}:\n" f"date: {doc.created_at:%Y-%m-%d}\n" + f"{marker}" f"{doc.content[:cap]}\n" f"{TRUNCATION_MARKER}\n" f"{READ_TRUNCATION_NOTICE.format(shown=cap, total=len(doc.content))}" ) # At or under the cap: the pre-phase-95 result plus the # phase-106 D5 date line (first line byte-identical — the - # mock's header contract; no marker, no notice, no holder - # entry, no ToolResultPiece). + # mock's header contract; no truncation marker, no notice, no + # holder entry, no ToolResultPiece) — and, phase 122, the + # image-document marker line for image docs only. return ( f"Document {doc.source}/{doc.path}:\n" f"date: {doc.created_at:%Y-%m-%d}\n" + f"{marker}" f"{doc.content}" ) if call.name == "grep": diff --git a/app/rag/importer.py b/app/rag/importer.py index 11992d0..1ff8d40 100644 --- a/app/rag/importer.py +++ b/app/rag/importer.py @@ -30,7 +30,12 @@ by their exact lowercased full filename (``Dockerfile`` under the no longer exist **or no longer match the format filter** — this is how previously-imported junk (e.g. dot-dir READMEs) leaves the index. Per-file logging uses the verbs ``added | updated | unchanged | pruned`` plus a -summary line with per-format counts (PLAN §9). +summary line with per-format counts (PLAN §9). Phase 122 prune guard +(LOCKED, derived from A3/A4): while the ``images`` toggle is OFF, an +``is_image`` doc is INVISIBLE to the walk, not a deleted file — prune +skips it (turning the toggle off and syncing must never destroy image +documents); a toggle-ON run prunes a deleted image file normally and +deletes its ``image_dir`` copy with the row. Document dates (phase 106, D2/D4): every import sources ``documents.created_at`` from the file's source — the per-file git @@ -54,6 +59,32 @@ backfill runs BEFORE the ``created_at_manual`` early-return (the manual flag protects the DATE only, D1) and the strict ``is None`` check leaves owner-set summaries (even empty strings, phase 57) alone. +Standalone images (phase 122, LOCKED A3): with the ``images`` toggle +(``BOR_IMAGES``) ON, the walk also admits the image extension set +(``BOR_IMAGE_EXTENSIONS`` — a SEPARATE set from ``import_extension_set``; +images are never user-added via ``BOR_IMPORT_EXTENSIONS``, the toggle is +the single knob). Such a file takes the binary index path +(:func:`_index_image_file`): the sha256 digest is over the raw BYTES +(content identity — the digest rule is unchanged), the bytes are copied +to the persistent home ``settings.image_dir/.`` (dir created +on demand; the copy is written ONLY after a successful description, so a +failure never leaves an orphan; a changed image deletes the stale copy +first; a pruned image doc deletes its copy), the row carries +``is_image=True`` + ``image_path``, and ``content`` is the vision +description — the ONLY embedded text of the document (the embedding model +never sees pixels; ``read_text`` is never called for an image). The +description comes through the single seam :func:`_describe_or_skip` +(task 03: :func:`app.rag.summarizer.describe_image` — ONE CHAT-model +(vision) call with the image bytes as a base64 data URL; the ``lite`` +summary model is NOT assumed vision-capable, LOCKED A3); a failed/empty +description SKIPS the doc entirely (no row, no copy) — counted in +``images_failed`` + a warning, the sync continues (fail-soft). The normal +chunk pipeline then embeds ``content`` and the phase-30 summary path runs +on it — image-aware (task 03): for an image doc the description IS the +summary (stored verbatim, no ``lite`` call, no pointer line), so the +``is_summary`` position −1 chunk mirrors ``Document.summary``, which +equals ``Document.content``. + ``import_sources`` accepts an optional per-file ``progress`` callback (phase 64, task 01) reporting the file being processed right now. """ @@ -61,11 +92,12 @@ from __future__ import annotations import hashlib import logging +import uuid from collections.abc import Callable from dataclasses import dataclass, field from datetime import UTC, datetime from pathlib import Path -from typing import Protocol +from typing import Any, Protocol from sqlalchemy import select from sqlalchemy.orm import Session @@ -76,7 +108,12 @@ from app.models import Chunk, Document from app.rag.chunker import chunk_document, extract_title from app.rag.doc_dates import file_mtime_datetime, normalize_doc_date from app.rag.llm import EmbeddingError, LLMError -from app.rag.summarizer import generate_summary +from app.rag.summarizer import ( + IMAGE_FALLBACK_MIME, + IMAGE_MIMES, + describe_image, + generate_summary, +) logger = logging.getLogger("app.importer") @@ -94,9 +131,12 @@ class Embedder(Protocol): async def embed(self, texts: list[str]) -> list[list[float]]: ... - async def chat(self, messages: list[dict[str, str]], model: str | None = None) -> str: ... - # ^ the one-shot completion the summarizer uses for the ``lite`` model - # (phase 30, task 01); :class:`app.rag.llm.LLMClient` satisfies it. + async def chat(self, messages: list[dict[str, Any]], model: str | None = None) -> str: ... + # ^ the one-shot completion the summarizer uses — the ``lite`` model + # for text summaries (phase 30, task 01) and the CHAT (vision) + # model for the phase-122 image description (multimodal content: + # a string or a list of OpenAI-compatible parts); :class:`app.rag. + # llm.LLMClient` satisfies it. @dataclass @@ -129,6 +169,12 @@ class ImportSummary: #: so no ``sources_meta`` bump, no overview/folder-summary #: regeneration). dates_updated: int = 0 + #: Image docs (phase 122, LOCKED A3) whose vision description failed + #: or came back empty — the doc is SKIPPED entirely (no row, no + #: ``image_dir`` copy): an undescribed image is unsearchable noise. + #: Fail-soft: the sync continues, this counter + the warning line + #: are the signal. + images_failed: int = 0 #: Files walked, keyed by lowercased extension (``md``, ``yaml``, …). formats: dict[str, int] = field(default_factory=dict) @@ -143,7 +189,7 @@ class ImportSummary: logger.info( "import: summary files=%d added=%d updated=%d unchanged=%d pruned=%d " "errors=%d chunks=%d embed_batches=%d summaries=%d summary_errors=%d " - "summary_backfilled=%d dates_updated=%d formats=%s", + "summary_backfilled=%d dates_updated=%d images_failed=%d formats=%s", self.files, self.added, self.updated, @@ -156,6 +202,7 @@ class ImportSummary: self.summary_errors, self.summary_backfilled, self.dates_updated, + self.images_failed, self.format_counts(), ) @@ -238,6 +285,7 @@ def iter_importable_files( excluded: frozenset[str] = EXCLUDED_DIRS, ignore: tuple[str, ...] = (), include_hidden: bool = False, + image_extensions: frozenset[str] = frozenset(), ) -> list[Path]: """All importable files under *root* (sorted), per the A9 scope rules. @@ -255,6 +303,13 @@ def iter_importable_files( source-relative POSIX path starts with any entry; the default ``()`` keeps every existing caller byte-identical. The *ignore* tuple composes additively in both states. + + *image_extensions* (phase 122) is the lowercased dotted + image-extension set admitted IN ADDITION to *extensions* — passed by + :func:`import_sources` only while the ``images`` toggle is on (it + reads ``llm.settings``; the image set is never merged into + *extensions*). The empty default admits nothing: every existing caller + (and the toggle-off walk) stays byte-identical to pre-phase. """ if not root.is_dir(): return [] @@ -270,7 +325,12 @@ def iter_importable_files( continue if ignore and is_ignored(rel.as_posix(), ignore): continue - if match_extension(path, extensions) is None: + matched = match_extension(path, extensions) + if matched is None and image_extensions: + # Phase 122: the image set is admitted IN ADDITION to the + # import set (toggle on only — the caller passes it in). + matched = match_extension(path, image_extensions) + if matched is None: continue files.append(path) return files @@ -340,10 +400,23 @@ async def import_sources( mtime fallback applies to every file — which IS the behavior change, D4: an unchanged file now refreshes its stored date from its source on every run (the backfill-correction case). + + Images (phase 122): when ``llm.settings.images`` is on, BOTH walks + (the progress pre-walk and the processing loop — same rules, so + ``total`` counts images) also admit ``llm.settings.image_extension_set`` + files, each indexed through the binary image path (see the module + docstring). ``prune=True`` with the toggle ON prunes a deleted image + file normally (row + ``image_dir`` copy); with the toggle OFF the + prune skips ``is_image`` docs (the prune guard — the image is + invisible to the walk, not a deleted file). """ if limit is not None and limit <= 0: raise ValueError("limit must be >= 1") summary = ImportSummary() + # Phase 122: the image set is admitted by the walks ONLY while the + # toggle is on — the empty set admits nothing, so the toggle-off run + # (walk, counts, prune) stays byte-identical to pre-phase. + image_exts = llm.settings.image_extension_set if llm.settings.images else frozenset() owns_session = session is None if session is None: session = SessionLocal() @@ -364,6 +437,7 @@ async def import_sources( include_hidden=_include_hidden_for_root( root, include_hidden_by_root ), + image_extensions=image_exts, ) ) try: @@ -386,6 +460,7 @@ async def import_sources( llm.settings.import_extension_set, ignore=ignore, include_hidden=include_hidden, + image_extensions=image_exts, ): if limit is not None and summary.files >= limit: break @@ -394,9 +469,11 @@ async def import_sources( summary.files += 1 # Phase 102: the matched bare token (``dockerfile`` for an # extensionless ``Dockerfile``), never ``unknown`` — the - # file is in scope, so the walk matched it. + # file is in scope, so the walk matched it. Phase 122: an + # image file matches the image set, not the import set. ext = ( match_extension(path, llm.settings.import_extension_set) + or match_extension(path, image_exts) or "unknown" ) summary.formats[ext] = summary.formats.get(ext, 0) + 1 @@ -423,7 +500,9 @@ async def import_sources( if limit is not None: logger.warning("import: --prune ignored because --limit was given") else: - summary.pruned = _prune(session, source_names, seen) + summary.pruned = _prune( + session, source_names, seen, images=llm.settings.images + ) summary.embed_batches = llm.embed_batches summary.log() return summary @@ -448,8 +527,24 @@ async def _index_file( git last-commit datetime from the caller's ``doc_dates_by_root`` map, or ``None`` (every non-git case): the file's mtime is read here, once, and becomes the source date (the D2 fallback). + + Image files (phase 122, toggle on) delegate to + :func:`_index_image_file` — the binary path (bytes digest, + persistent copy, ``content`` = the vision description) — BEFORE + any text read: ``read_text`` is never called for an image. """ settings = llm.settings + # Phase 122 (task 02): the image branch FIRST. Only reachable while + # the ``images`` toggle is on — the walk never admits image files + # while it is off, and with it off this check is a no-op (the text + # path below stays byte-identical to pre-phase). + if settings.images: + image_set = settings.image_extension_set + if match_extension(full_path, image_set) is not None: + return await _index_image_file( + session, source=source, rel=rel, full_path=full_path, llm=llm, + summary=summary, raw_date=raw_date, + ) content = full_path.read_text(encoding="utf-8", errors="replace").replace("\x00", "") digest = hashlib.sha256(content.encode("utf-8")).hexdigest() doc = session.scalar(select(Document).where(Document.source == source, Document.path == rel)) @@ -609,9 +704,24 @@ async def _store_summary( a success counts ``summary_backfilled`` instead of ``summaries`` (the doc content is untouched, so the import's KB-change signal must not move); the rest of the mechanics are identical. + + Image docs (phase 122, task 03, LOCKED A3): for an ``is_image`` doc + the vision description — ``content`` (which equals ``doc.content`` + on the backfill path) — IS the summary: no ``lite`` call, no + pointer line (the summary mirrors the description verbatim, so + ``doc.summary`` == ``doc.content``). The phase-30 chunk mechanics + (one ``is_summary`` position −1 chunk, replacement, best-effort + rollback) are unchanged; the only remaining failure class is the + summary chunk's embed (the ``doc`` row + content chunks survive — + fail-soft, same as the text path). """ try: - text = await generate_summary(llm, source=source, path=rel, content=content) + if doc.is_image: + # Phase 122 (task 03): the description IS the summary — + # stored verbatim (no ``lite`` call, no pointer line). + text = content + else: + text = await generate_summary(llm, source=source, path=rel, content=content) # Replacement: at most one summary chunk per document at a time. # Removing from the collection is what the ``delete-orphan`` # cascade turns into a row delete on flush — and it keeps the @@ -646,14 +756,265 @@ async def _store_summary( logger.error("import: summary failed source=%s path=%s — %s", source, rel, e) -def _prune(session: Session, source_names: set[str], seen: set[tuple[str, str]]) -> int: - """Delete documents of *source_names* whose file is no longer in *seen*.""" +async def _describe_or_skip( + llm: Embedder, *, data: bytes, source: str, rel: str, full_path: Path +) -> str | None: + """Phase 122 — the SINGLE seam for the image description (task 03). + + Returns the vision description that becomes the image document's + ``content`` (and ``summary`` — the ONLY embedded text of the doc), + or ``None`` when the description failed or came back empty — the + caller then SKIPS the doc entirely (no row, no copy) and counts + ``summary.images_failed`` (LOCKED A3, fail-soft: the sync continues, + the warning + counter are the signal). + + The seam is one line by design (the importer's tests patch exactly + this function): :func:`app.rag.summarizer.describe_image` — ONE + CHAT-model (vision) call (LOCKED A3) with the bytes as a data URL + whose mime comes from :data:`app.rag.summarizer.IMAGE_MIMES` + (dotted extension; an unlisted ``BOR_IMAGE_EXTENSIONS`` token takes + the generic fallback — a rejection there fails soft like any other + description error). ``source``/``rel`` stay on the signature so the + caller (and the patch) reads like the document being described; + the failure's doc identity is logged by the caller's warning. + """ + mime = IMAGE_MIMES.get(full_path.suffix.lower(), IMAGE_FALLBACK_MIME) + return await describe_image(llm, data=data, mime=mime) + + +def _delete_image_copy(image_path: str | None) -> None: + """Best-effort removal of a stale image copy (phase 122). + + A missing path is a no-op (already gone — e.g. the owner cleaned + the image dir); an unreadable one is logged, never raised — copy + cleanup must not break the sync (the doc row's fate is decided by + the upsert/prune logic, not by filesystem hygiene). + """ + if not image_path: + return + try: + Path(image_path).unlink(missing_ok=True) + except OSError as e: + logger.warning("import: could not delete image copy %s — %s", image_path, e) + + +async def _index_image_file( + session: Session, + *, + source: str, + rel: str, + full_path: Path, + llm: Embedder, + summary: ImportSummary, + raw_date: datetime | None = None, +) -> None: + """The phase-122 image branch of :func:`_index_file` — a standalone + image is indexed from its BYTES, never its text: + + * the sha256 digest is over the raw bytes (the digest rule is + content identity — the same bytes are the same document); + * the PERSISTENT copy lands in ``settings.image_dir`` as + ``.`` (the dir is created on demand; the copy is + written only AFTER a successful description, so a failure never + leaves an orphan; a changed image deletes the stale copy before + replacing it); + * ``content`` is the vision description — the ONLY embedded text of + the document (the embedding model never sees pixels) — and the + normal chunk pipeline then embeds it, with the phase-30 summary + path running on it (the ``is_summary`` position −1 chunk mirrors + ``Document.summary``). + + ``raw_date`` follows the text path exactly (the phase-106 D2 + fallback: no source date in the map → the file's mtime, read before + the unchanged early-return because the unchanged path refreshes the + stored date from the same source; the D1 manual-date lock and the + D4 refresh apply unmodified). + + Fail-soft (LOCKED A3): a failed/empty description SKIPS the doc + entirely (no row, no copy) — ``summary.images_failed`` + a warning, + the sync continues. + """ + settings = llm.settings + data = full_path.read_bytes() + digest = hashlib.sha256(data).hexdigest() + doc = session.scalar(select(Document).where(Document.source == source, Document.path == rel)) + if raw_date is None: + # D2 fallback (same as the text path): no source date in the map + # → the file's mtime (one stat). + raw_date = file_mtime_datetime(full_path) + + if doc is not None and doc.content_hash == digest: + # Unchanged image (byte digest) — the text path's unchanged + # branch, unmodified in shape. + summary.unchanged += 1 + logger.info("import: unchanged source=%s path=%s", source, rel) + # Phase 118 (A2) backfill, image flavour: an unchanged image doc + # whose summary is still NULL (an earlier fail-soft summary miss + # — for an image, the one remaining failure class: the summary + # chunk's embed) gets the same best-effort summary pass. For an + # image the summary IS the stored description (``doc.content``), + # so the image-aware ``_store_summary`` (task 03) re-stores it + # verbatim with one ``is_summary`` chunk; a failure (the + # embed) keeps the doc as-is (no row mutation) — fail-soft, + # same as the text path. + if doc.summary is None: + await _store_summary( + session, doc=doc, source=source, rel=rel, content=doc.content, + llm=llm, summary=summary, backfill=True, + ) + if doc.created_at_manual: + # D1/D4: the owner's correction survives the sync — no write + # at all (the text path's manual-date early-return). + return + # D4: the date refreshes on every sync, including unchanged + # files, and may go OLDER (no monotonic guard). + target = normalize_doc_date(raw_date) + if target != doc.created_at: + doc.created_at = target + session.commit() + summary.dates_updated += 1 + logger.info( + "import: date-refreshed source=%s path=%s date=%s", + source, rel, doc.created_at.isoformat(), + ) + return + + verb = "updated" if doc is not None else "added" + # The description is the doc's content (task 03: and its summary) — + # it is generated BEFORE anything is written, so a failure skips the + # doc with no row and no copy (the copy is only made after a + # successful description — a failure never leaves an orphan). + content = await _describe_or_skip( + llm, data=data, source=source, rel=rel, full_path=full_path + ) + if content is None: + # LOCKED A3 fail-soft: an undescribed image is unsearchable + # noise — skip the doc entirely (no row, no copy). + summary.images_failed += 1 + logger.warning("import: image description failed source=%s path=%s", source, rel) + return + + # The persistent copy: uploads are replaced on every upload, git + # checkouts are re-cloned, local dirs are user-edited — the served + # bytes must outlive the source file. Named by the doc id: a new + # doc's id is the uuid4 chosen here (row and copy agree); a changed + # doc keeps its id (the copy path is stable). + doc_id = doc.id if doc is not None else uuid.uuid4() + image_dir = Path(settings.image_dir).expanduser() + image_dir.mkdir(parents=True, exist_ok=True) + if doc is not None: + # A CHANGED image (hash differs): the stale copy is deleted + # before replacement. + _delete_image_copy(doc.image_path) + copy_path = image_dir / f"{doc_id}{full_path.suffix.lower()}" + copy_path.write_bytes(data) + + # The non-markdown title rule (the image's content is prose, but the + # doc IS the image — the file stem is the title). + title = full_path.stem + if doc is None: + doc = Document( + id=doc_id, + source=source, + path=rel, + full_path=str(full_path), + title=title, + content=content, + content_hash=digest, + indexed_at=datetime.now(UTC), + created_at=normalize_doc_date(raw_date), + is_image=True, + image_path=str(copy_path), + ) + session.add(doc) + else: + doc.full_path = str(full_path) + doc.title = title + doc.content = content + doc.content_hash = digest + doc.indexed_at = datetime.now(UTC) + # Phase 106 (D4): a content change is a new document version — + # the date is re-sourced and a previous manual correction is + # reset (it referred to the old content). + doc.created_at = normalize_doc_date(raw_date) + doc.created_at_manual = False + doc.is_image = True + doc.image_path = str(copy_path) + + session.flush() # guarantees doc.id even for brand-new rows + + # Phase 1+2 — the UNCHANGED pipeline on the description: chunk, + # replace the chunk rows (embeddings NULL), embed, and commit the + # whole file atomically (one transaction per file). The token-cap + # retry loop is copied from the text path; a description is short, + # so it never fires in practice. + target = max(400, settings.chunk_target_chars) + while True: + chunks_text = chunk_document(content, rel, target, settings.chunk_overlap_chars) + doc.chunks = [ + Chunk(document_id=doc.id, position=i, content=c) for i, c in enumerate(chunks_text) + ] + session.flush() # delete-orphan cascade drops the previous rows + if not doc.chunks: + break + try: + vectors = await llm.embed([c.content for c in doc.chunks]) + for row, vec in zip(doc.chunks, vectors, strict=True): + row.embedding = vec + break + except EmbeddingError as e: + if "token cap" not in str(e) or target <= 400: + raise + logger.info( + "import: re-chunking at %d chars after endpoint token cap: %s", + target // 2, + rel, + ) + target //= 2 + session.commit() + + if verb == "added": + summary.added += 1 + else: + summary.updated += 1 + summary.chunks += len(chunks_text) + logger.info("import: %s source=%s path=%s chunks=%d", verb, source, rel, len(chunks_text)) + + # Phase 30 shape on the description — image-aware (task 03, LOCKED + # A3): the description IS the summary (stored verbatim, no + # ``lite`` call), so ``doc.summary`` == ``doc.content`` and the + # ``is_summary`` position −1 chunk mirrors it. + await _store_summary( + session, doc=doc, source=source, rel=rel, content=content, llm=llm, summary=summary + ) + + +def _prune( + session: Session, + source_names: set[str], + seen: set[tuple[str, str]], + images: bool = False, +) -> int: + """Delete documents of *source_names* whose file is no longer in *seen*. + + *images* (phase 122 prune guard, LOCKED, derived from A3/A4): while + the image toggle is OFF (``images`` False), every ``is_image`` doc is + SKIPPED — the image is invisible to an images-off walk, not a deleted + file, so pruning it would silently destroy image documents on the + first images-off sync. Toggle ON → normal semantics: a deleted image + file prunes its doc, and the pruned image's ``image_dir`` copy is + deleted with it. + """ if not source_names: return 0 pruned = 0 docs = session.scalars(select(Document).where(Document.source.in_(source_names))).all() for doc in docs: if (doc.source, doc.path) not in seen: + if doc.is_image and not images: + # Prune guard: invisible to the walk, not deleted. + continue + _delete_image_copy(doc.image_path) session.delete(doc) pruned += 1 logger.info("import: pruned source=%s path=%s", doc.source, doc.path) diff --git a/app/rag/retriever.py b/app/rag/retriever.py index 5bb5e92..1c9b78c 100644 --- a/app/rag/retriever.py +++ b/app/rag/retriever.py @@ -92,6 +92,7 @@ from sqlalchemy.orm import Session from app.config import get_settings from app.models import Chunk, Document +from app.schemas import SourceRef #: Shared overflow marker (phase 15; imported by ``app.rag.prompts``) #: — used by the steering () section, the phase-118 NULL-summary @@ -939,3 +940,27 @@ def select_related( continue out.append(doc) return out + + +def source_ref_with_image(doc: Document) -> SourceRef: + """One :class:`~app.schemas.SourceRef` wire frame for *doc* — the + phase-122 (task 05) SHARED frame builder: the chat API's cited + tier (the agent's read docs) and the related tier both run through + it, so the per-doc ref shape has exactly one construction site. + + The ref carries the chip identity (``source`` / ``path`` / + ``title``) and, ONLY for a standalone-image document (``is_image``), + the optional ``image_url`` — the image BYTES route + ``/api/documents//image`` (the chat's sources block renders the + compact inline image from it, the summary as alt + caption — + "shown in the chat nicely", TODO L6). For a TEXT document the field + stays ``None`` and is DROPPED by the model's serializer (never + ``null`` — the omission rule): a text-doc frame is byte-identical + to pre-phase. The frame's doc id rides the path — the same way the + document content endpoint's ``(source, path)`` lookup does (no new + id leak beyond what the frame already carries). + """ + ref = SourceRef(source=doc.source, path=doc.path, title=doc.title) + if doc.is_image: + ref.image_url = f"/api/documents/{doc.id}/image" + return ref diff --git a/app/rag/summarizer.py b/app/rag/summarizer.py index 6a2604e..214a861 100644 --- a/app/rag/summarizer.py +++ b/app/rag/summarizer.py @@ -1,4 +1,5 @@ -"""Document summarizer (phase 30, task 03). +"""Document summarizer (phase 30, task 03) + image descriptions +(phase 122, task 03). Builds the ``SUMMARY_MODE`` prompt for one document, calls the aipi ``lite`` model through the one-shot ``LLMClient.chat`` (phase 30, @@ -22,18 +23,33 @@ Quality contracts enforced here: summarizer re-asserts defensively and never hands the importer a pointer-only row). -The ``SUMMARY_MODE`` marker follows the ``DEFLECT_MODE`` convention: -the deterministic E2E mock LLM keys on it in the system prompt -(``tests/e2e/mock_llm.py`` — wired in task 06). +Image descriptions (phase 122, LOCKED A3): :func:`describe_image` is +this module's second one-shot generation path — a SINGLE CHAT-model +(vision) call describing one image's bytes as a base64 data URL. The +description becomes the image document's ``content`` AND ``summary`` +(it is the ONLY embedded text of the doc — the embedding model never +sees pixels, and the ``lite`` summary model is deliberately NOT used: +it is not assumed vision-capable). Fail-soft by contract: any client +error, empty reply, or non-2xx yields ``None`` — the importer skips +the doc, counts ``images_failed``, and the sync continues. + +The ``SUMMARY_MODE`` / ``IMAGE_DESCRIPTION_MODE`` markers follow the +``DEFLECT_MODE`` convention: the deterministic E2E mock LLM keys on +them (``tests/e2e/mock_llm.py`` — the image branch is wired by task +06's story suite). """ from __future__ import annotations -from typing import Protocol +import base64 +import logging +from typing import Any, Protocol from app.config import Settings, get_settings from app.rag.llm import LLMError from app.rag.retriever import TRUNCATION_MARKER +logger = logging.getLogger("app.summarizer") + #: System-prompt marker for summary generation — the E2E mock LLM keys on #: it (same convention as ``DEFLECT_MODE``, PLAN §6). SUMMARY_MODE = "SUMMARY_MODE" @@ -51,6 +67,55 @@ SUMMARY_INSTRUCTION = ( #: Full system prompt: marker first (the mock's key), then the instruction. SYSTEM_PROMPT = f"{SUMMARY_MODE}: {SUMMARY_INSTRUCTION}" +#: Image-description marker (phase 122, task 03) — the deterministic +#: E2E mock LLM keys on it (same convention as ``SUMMARY_MODE`` / +#: ``DEFLECT_MODE``, PLAN §6; the story suite wires the mock's branch +#: in task 06). It heads the text part of the multimodal describe +#: message, so it rides the user message, not a system prompt. +IMAGE_DESCRIPTION_MODE = "IMAGE_DESCRIPTION_MODE" + +#: Locked instruction for the CHAT (vision) model (phase 122, LOCKED +#: A3): the description is the ONLY retrievable text of the image +#: document (the embedding model never sees pixels), so it must be a +#: faithful, retrieval-oriented account that carries the image's full +#: meaning — what is depicted, any visible text/labels/titles, +#: diagram/table structure, salient details. +DESCRIBE_INSTRUCTION = ( + "Describe this image faithfully, in plain text, for a search index. " + "State what is depicted, transcribe any visible text, labels, or " + "titles, describe the structure of any diagram, table, or layout, and " + "call out the most salient details. Write 2-4 sentences of substance. " + "Do not use markdown. Do not invent anything that is not visible in " + "the image. Your description is the ONLY text that will ever be " + "retrieved for this image — it must carry the image's full meaning." +) + +#: The full describe prompt: marker first (the mock's key), then the +#: instruction — the single text part of the multimodal user message. +DESCRIBE_PROMPT = f"{IMAGE_DESCRIPTION_MODE}: {DESCRIBE_INSTRUCTION}" + +#: Extension → MIME type for the image family (phase 122). Dotted, +#: lowercase keys — the ``image_extension_set`` shape. Task 03 uses it +#: for the describe call's data-URL mime; task 04's serve route reuses +#: it for the image bytes' ``Content-Type`` (one map, one truth). A +#: ``BOR_IMAGE_EXTENSIONS`` token outside this map (a custom format) +#: takes the :data:`IMAGE_FALLBACK_MIME` data-URL mime in the describe +#: call — the vision endpoint may reject it, and the fail-soft skip +#: (``images_failed``) is the honest outcome. +IMAGE_MIMES: dict[str, str] = { + ".png": "image/png", + ".jpg": "image/jpeg", + ".jpeg": "image/jpeg", + ".webp": "image/webp", + ".gif": "image/gif", + ".bmp": "image/bmp", +} + +#: The data-URL mime for an image extension :data:`IMAGE_MIMES` does not +#: name (phase 122) — the best-effort generic, never a guess at a +#: specific type. +IMAGE_FALLBACK_MIME = "application/octet-stream" + class SummaryLLM(Protocol): """The one-shot chat surface the summarizer needs. @@ -58,12 +123,17 @@ class SummaryLLM(Protocol): :class:`app.rag.llm.LLMClient` satisfies it; unit tests pass a duck-typed fake (``chat`` + ``settings``) instead — same pattern as the importer's ``Embedder`` protocol. + + ``content`` may be a string (text calls — ``generate_summary``) or + a list of OpenAI-compatible parts (phase 122 multimodal image + descriptions — ``{type: "text", …}`` + ``{type: "image_url", …}``); + the client passes message dicts through untouched. """ settings: Settings async def chat( - self, messages: list[dict[str, str]], model: str | None = None + self, messages: list[dict[str, Any]], model: str | None = None ) -> str: ... @@ -121,3 +191,58 @@ async def generate_summary( "refusing to store a silent summary" ) return f"{summary}\nSource: {source}/{path}" + + +async def describe_image( + llm: SummaryLLM, + *, + data: bytes, + mime: str, + settings: Settings | None = None, +) -> str | None: + """One-shot CHAT-model (vision) description of one image (phase 122, + LOCKED A3) — the text that becomes the image document's ``content`` + AND ``summary`` (the ONLY embedded text of the doc; the embedding + model never sees pixels). + + ONE chat-model call against ``settings.llm_chat_model`` (the vision + model — the ``lite`` summary model is NOT assumed vision-capable) + with the multimodal user message the OpenAI-compatible API expects: + ``[{type: "text", text: DESCRIBE_PROMPT}, {type: "image_url", + image_url: {url: }}]`` — no system + prompt, no tools, no app-level retries beyond the client's own + (SDK-level + the house one-shot empty-content policy) — a + description failure must not stall a sync. + + Returns the stripped reply cut exactly at + ``(settings or llm.settings).summary_max_chars`` (the phase-30 cap — + the description IS the summary, so it keeps the same uniform + ceiling). Returns ``None`` on any client error, empty reply, or + non-2xx (the client raises :class:`LLMError` for all three classes) + — the caller (the importer's ``_describe_or_skip`` seam) fails soft: + the doc is skipped, counted in ``images_failed``, and the sync + continues (LOCKED A3). + """ + data_url = f"data:{mime};base64,{base64.b64encode(data).decode('ascii')}" + messages: list[dict[str, Any]] = [ + { + "role": "user", + "content": [ + {"type": "text", "text": DESCRIBE_PROMPT}, + {"type": "image_url", "image_url": {"url": data_url}}, + ], + } + ] + model = llm.settings.llm_chat_model + try: + raw = await llm.chat(messages, model=model) + except LLMError as e: + logger.warning("image description failed (model=%s): %s", model, e) + return None + text = raw.strip() + if not text: + # The client already rejects empty content; this is the + # defensive re-assert (the duck-typed fakes may return it). + return None + limit = (settings or llm.settings).summary_max_chars + return text[:limit] diff --git a/app/schemas.py b/app/schemas.py index 72a8067..b896416 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -106,11 +106,36 @@ class SourceRef(BaseModel): rows server-side, so every server-built SSE ref fits by construction (A3: the SSE path is provably unaffected); the cap binds only client-saved refs — bounded at the boundary with a 422. + + Phase 122 (task 05): ``image_url`` — the image BYTES route + (``/api/documents//image``) for a ref whose document is a + standalone image: the chat's sources block renders the compact + inline image from it (the "shown in the chat nicely" contract, TODO + L6). It is the ONLY new frame field (the doc id rides the path — + the same way the document content endpoint's ``(source, path)`` + lookup does). For a TEXT document the field stays ``None`` and is + DROPPED on serialization (never ``null`` — the :class:`DocContent` + omission precedent), so a text-doc frame is byte-identical to + pre-phase. Server-built refs go through the shared + :func:`app.rag.retriever.source_ref_with_image` (one shape, both + frame tiers); a client-saved ref without the field parses with the + ``None`` default (pre-phase saved chats restore unchanged). """ source: str = Field(max_length=120) path: str = Field(max_length=1000) title: str = Field(max_length=500) + #: Phase 122 (task 05) — see the class docstring. ``None`` (every + #: text doc, and every pre-phase client-saved ref) is omitted on + #: serialization — the key is ABSENT, never ``null``. + image_url: str | None = None + + @model_serializer(mode="wrap") + def _serialize(self, handler: SerializerFunctionWrapHandler) -> Any: + data = handler(self) + if self.image_url is None: + data.pop("image_url", None) + return data class ChatThinkingEvent(BaseModel): @@ -275,6 +300,20 @@ class KbTreeFile(BaseModel): ``GET /api/docs`` returns) / ``created_at`` (phase 106, D8 — the document's creation date) / ``indexed_at`` (ISO-8601) are verbatim from the catalogue row the endpoint reads. + + Image affordance (phase 122, task 04): for an ``is_image`` file node + the three ``is_image`` / ``image_url`` / ``summary`` keys ride the + node (the RAG view's Path cell renders the 48px thumbnail from + ``image_url`` with ``alt = summary``). For a TEXT file node all + three are OMITTED from the wire shape (the + :func:`_drop_image_fields` omission rule — a pre-phase KB, which has + no image rows, serializes byte-identically to pre-phase, and the + RAG view reads ``is_image === true`` — it never expects the keys on + a text node). ``image_url`` is ``None``-omitted even on an image + node (a row whose ``image_path`` was lost renders the glyph + fallback); ``summary`` stays ``null`` on an image node (the alt + falls back to the title client-side — the fail-soft backfill + corner). """ kind: Literal["file"] = "file" @@ -286,6 +325,23 @@ class KbTreeFile(BaseModel): #: the ``Created`` column (before ``Indexed``). created_at: str indexed_at: str + #: Phase 122 (task 04) — true iff the file is an image document + #: (LOCKED A3). Omitted from a text node's wire shape (see the class + #: docstring); the builder sets it only for a node whose + #: ``(source, path)`` is in the endpoint's image-docs map. + is_image: bool = False + #: Phase 122 (task 04) — the image bytes route + #: (``/api/documents//image``) for the RAG view's thumbnail; + #: ``None`` (→ absent) when the row has no servable copy. + image_url: str | None = None + #: Phase 122 (task 04) — the document's summary (for an image doc, + #: the vision description — the thumbnail's ``alt``); ``None`` for a + #: fail-soft row still awaiting the backfill. + summary: str | None = None + + @model_serializer(mode="wrap") + def _serialize(self, handler: SerializerFunctionWrapHandler) -> Any: + return _drop_image_fields(self, handler) class KbTreeFolder(BaseModel): @@ -374,8 +430,11 @@ class KbTree(BaseModel): 98, D3): true iff its recursive document count ≥ ``MIN_DOCS_PER_FOLDER`` (1) AND it has no stored ``folder_summaries`` row — exactly ``missing_folder_summaries``'s candidate set (the - marker never drifts from the gap-fill); FILE nodes carry no flag - (the file table has no description column). + marker never drifts from the gap-fill). FILE nodes carry no pending + flag (the file table has no description column) — but, since phase + 122 (task 04), an image FILE node carries the thumbnail affordance + keys (``is_image`` / ``image_url`` / ``summary`` — omitted on text + nodes, see :class:`KbTreeFile`). """ sources: list[KbTreeSource] @@ -400,6 +459,27 @@ class DocContent(BaseModel): content: str indexed_at: str chunks: int + #: Phase 122 (task 04) — true iff the document is a standalone + #: image (LOCKED A3: ``content`` is the vision description, the + #: bytes live behind :attr:`image_url`). ALWAYS present on the wire + #: (text docs: ``false`` — the wire-additive key, the phase-106 + #: ``created_at`` pattern); the viewer renders the ```` block + #: only when true. + is_image: bool = False + #: Phase 122 (task 04) — the image bytes route + #: (``/api/documents//image``) for the viewer's ````. + #: ABSENT from the wire for text docs (``None`` → dropped by the + #: serializer — never ``null``, the :func:`_drop_absent_share_url` + #: omission precedent); also absent for an image row whose + #: ``image_path`` is NULL (the viewer's onerror fallback covers it). + image_url: str | None = None + + @model_serializer(mode="wrap") + def _serialize(self, handler: SerializerFunctionWrapHandler) -> Any: + data = handler(self) + if self.image_url is None: + data.pop("image_url", None) + return data class SummaryUpdate(BaseModel): @@ -891,6 +971,25 @@ class SavedChatUpdate(BaseModel): messages: list[ChatMessage] = Field(min_length=1, max_length=200) +def _drop_image_fields(model: KbTreeFile, handler: SerializerFunctionWrapHandler) -> Any: + """The phase-122 (task 04) image-affordance omission rule for + :class:`KbTreeFile` file nodes: a TEXT node (``is_image`` false) drops + ALL three image keys — a pre-phase KB (no image rows) serializes + byte-identically to pre-phase, and the RAG view's file row stays the + pre-phase bare-link cell. An IMAGE node keeps ``is_image`` + + ``summary`` (a ``null`` summary is meaningful — the alt falls back + client-side) and drops ``image_url`` only when ``None`` (the + row-without-a-copy corner — never a ``null`` on the wire, the + :func:`_drop_absent_share_url` precedent).""" + data = handler(model) + if not model.is_image: + for key in ("is_image", "image_url", "summary"): + data.pop(key, None) + elif data.get("image_url") is None: + data.pop("image_url", None) + return data + + def _drop_absent_share_url(model: BaseModel, handler: SerializerFunctionWrapHandler) -> Any: """The ``share_url`` omission rule (phase 51, task 02): ``None`` → ABSENT from the JSON (not ``"share_url": null``) — an unshared chat diff --git a/frontend/assets/app.js b/frontend/assets/app.js index bafdca2..2e5cbfc 100644 --- a/frontend/assets/app.js +++ b/frontend/assets/app.js @@ -1526,6 +1526,10 @@ function appendSources(wrap, sources) { chip.textContent = label; chip.title = label; meta.appendChild(chip); + // Phase 122 (task 05): a ref for an IMAGE document carries + // `image_url` (the frame's only new field — omitted on text refs): + // its chip gains the compact inline figure right after it. + if (s.image_url) appendSourceImageFigure(meta, s); } body.appendChild(meta); // Accessible full path whenever the pill visually truncates. @@ -1575,10 +1579,82 @@ function appendRelated(wrap, related) { link.title = docLabel; // full path as the native tooltip (chip pattern) link.setAttribute("aria-label", docLabel); // the accessible name is the full path row.appendChild(link); + // Phase 122 (task 05): the related tier rides the same ref shape — + // an image doc's ref carries image_url and gets the same figure. + if (s.image_url) appendSourceImageFigure(row, s); } body.appendChild(row); } +/* Phase 122 (task 05): the chat's sources block shows a retrieved + * IMAGE document "nicely" (TODO L6): the ref's chip gains a COMPACT + * INLINE FIGURE right after it. The chip keeps its text and its + * affordance — the figure is ADDITIVE, never a replacement — and the + * figure reuses the chip's navigation (the same documentUrl href, the + * /document.html escape hatch; the same left-click → same-page modal, + * phase 26). alt + the VISIBLE caption = the document's SUMMARY (the + * vision description — the WCAG alt contract): the frame carries no + * summary (image_url is the only new frame field), so the figure + * fetches the content endpoint the chip's modal already uses (the + * (source, path) pair is the same lookup key) and swaps the summary + * in; until it settles — and when it fails — the title stands in (a + * caption is ALWAYS visible). A FAILED IMAGE LOAD collapses the figure + * to the plain chip (the figure is removed — never a broken-image + * icon: the row's bytes route 404s when the copy was lost). */ +function appendSourceImageFigure(meta, s) { + const label = `${s.source}/${s.path}`; + const fig = document.createElement("a"); + fig.className = "source-image"; + fig.setAttribute("role", "listitem"); + fig.href = documentUrl(s.source, s.path, "/"); // back → the chat page + fig.title = label; // full path as the native tooltip (chip pattern) + fig.setAttribute("aria-label", label); // the accessible name is the full path + fig.addEventListener("click", (e) => { + e.preventDefault(); // no new tab (phase 26) — the modal takes over + e.stopPropagation(); + openDocumentModal(s.source, s.path, fig); + }); + const img = document.createElement("img"); + img.className = "source-image-img"; + img.src = s.image_url; + img.alt = s.title || label; // the summary arrives via the fetch below + img.addEventListener("error", () => { + // The plain chip stays (it is the collapse target) — the figure, + // with its not-yet-resolved alt, goes. + fig.remove(); + }); + const caption = document.createElement("span"); + caption.className = "source-image-caption"; + caption.textContent = s.title || label; // visible caption: the title first… + fig.append(img, caption); + meta.appendChild(fig); + // …and the document's summary once the content fetch settles. + fetchContentSummary(s.source, s.path).then((summary) => { + if (!fig.isConnected) return; // collapsed (img error) or bubble cleared + if (summary && summary.trim() !== "") { + img.alt = summary; + caption.textContent = summary; + } + }); +} + +/* The document content endpoint the chip's modal already boots + * against (the (source, path) lookup key the ref carries) — phase 122 + * (task 05) asks it ONLY for the image figure's summary (alt + + * caption). Any failure (404, network, bad body) resolves to null — + * the title fallback stands and the figure is unaffected. */ +function fetchContentSummary(source, path) { + const url = + "/api/documents/content?source=" + + encodeURIComponent(source) + + "&path=" + + encodeURIComponent(path); + return fetch(url) + .then((r) => (r.ok ? r.json() : null)) + .catch(() => null) + .then((doc) => (doc && typeof doc.summary === "string" ? doc.summary : null)); +} + /* "Maybe try:" chips under a deflected bubble (honesty gate, phase 04, shared component + one-tap submit, phase 05). The group is accessible (role=list + aria-label) and wraps cleanly at every width. */ diff --git a/frontend/assets/document.js b/frontend/assets/document.js index ddcd116..9ee5a1e 100644 --- a/frontend/assets/document.js +++ b/frontend/assets/document.js @@ -102,6 +102,22 @@ * call — the phase-57 split). ONE shared core — the modal and * /document.html both get it (document-modal.js imports * renderDocument from this module — no per-surface copy). + * + * Phase 122 (task 04): image documents — when doc.is_image is true, + * #doc-content renders the persistent bytes FIRST (the block + * from doc.image_url — the /api/documents/{id}/image route — alt = + * the summary, the WCAG alt contract; a NULL summary falls back to + * the title), and the description (doc.content — the ONLY readable + * text of the doc) follows in the EXISTING plain-content slot below + * (the .doc-raw path; a description is prose, not markdown). The + * labeled Summary panel is suppressed for the verbatim-description + * case (summary === content — the importer invariant; the panel + * would duplicate the text right below the image); an admin-edited + * summary (different text) still renders in its panel with the + * phase-57 edit affordance. An load failure (the route's 404 — + * the row exists but the copy was lost) swaps the block for a small + * "Image unavailable" note (role=status): the page still shows the + * description. ONE shared core — page + modal both get it. */ import { bindSharedHeaderControls, fetchIsAdmin, initSharedHeader } from "./header.js"; @@ -189,12 +205,25 @@ export function renderDocument(doc, { titleEl, metaEl, contentEl }) { }); contentEl.replaceChildren(); + // Phase 122 (task 04): an image document — the persistent bytes + // render as the block FIRST; the description (= doc.content) + // follows in the normal content slot below (the plain-content path + // — the last branch in this function). + if (doc.is_image) { + contentEl.appendChild(docImageBlock(doc)); + } // Phase 36: the summary panel — labeled section ABOVE the original // content, on BOTH surfaces (page + modal) through this one core. // Only a non-empty summary renders: markdown docs carry none (phase // 30) and the fail-soft path leaves summary NULL, so both are - // byte-for-byte unchanged here. - if (doc.summary && doc.summary.trim() !== "") { + // byte-for-byte unchanged here. Phase 122 (task 04): for an IMAGE + // doc whose summary IS the verbatim description (summary === + // content — the importer invariant), the panel would duplicate the + // text right below the image, so it is suppressed; an admin-edited + // summary (different text) still renders with the phase-57 edit + // affordance. + const imageSummaryIsContent = doc.is_image && doc.summary === doc.content; + if (doc.summary && doc.summary.trim() !== "" && !imageSummaryIsContent) { const section = document.createElement("section"); section.className = "doc-summary"; section.setAttribute("aria-label", "Summary"); @@ -228,6 +257,40 @@ export function renderDocument(doc, { titleEl, metaEl, contentEl }) { } } +/* ---------- image block (phase 122, task 04) ---------- + * The viewer's image block for an ``is_image`` document: the + * document's PERSISTENT bytes (doc.image_url — the + * /api/documents/{id}/image route) as a block (max-width 100%; + * the theme's surface treatment lives in .doc-image). alt = the + * summary (the vision description — the WCAG alt contract everywhere); + * a NULL summary (the fail-soft backfill corner) falls back to the + * title. On a load failure (the route's 404 — the row exists but the + * copy was lost) the block shows a small "Image unavailable." + * note (role=status) in its place; the description in the content + * slot below still renders (the doc is still readable). Properties + * only (src, alt) — every document-derived value is a text node / + * property, never innerHTML (the XSS contract, unchanged). */ +function docImageBlock(doc) { + const wrap = document.createElement("div"); + wrap.className = "doc-image"; + const img = document.createElement("img"); + img.className = "doc-image-img"; + img.src = doc.image_url; + img.alt = + typeof doc.summary === "string" && doc.summary.trim() !== "" + ? doc.summary + : doc.title; + img.addEventListener("error", () => { + const note = document.createElement("p"); + note.className = "doc-image-unavailable"; + note.setAttribute("role", "status"); + note.textContent = "Image unavailable — the file is missing."; + wrap.replaceChildren(note); + }); + wrap.appendChild(img); + return wrap; +} + /* ---------- summary editing (phase 57, task 02 — D4, admin-only) ---------- * The .doc-summary panel is the ONE place the stored summary is edited * (page + modal through this core). Only an admin (docAdminReady) ever diff --git a/frontend/assets/sources.js b/frontend/assets/sources.js index d4c0ec3..b5cc3b3 100644 --- a/frontend/assets/sources.js +++ b/frontend/assets/sources.js @@ -285,6 +285,21 @@ * • the stat cards are UNTOUCHED — they keep their indexed_at * "last indexed" semantics (the owner asked for the column, not * the cards). + * + * Phase 122 (task 04) — the image-doc thumbnail: a file node of the + * tree carries the image affordance (is_image / image_url / summary — + * OMITTED on text nodes, the wire-additive rule) when it is an image + * document. makeRow's Path cell then renders a FIXED 48px thumbnail + * box (object-fit: cover, loading="lazy", alt = the summary — the + * vision description; a NULL summary falls back to the title) BEFORE + * the path link (the .kb-doc-path flex wrapper — the link keeps its + * ellipsis). Progressive enhancement: a failed fetch (or the lazy + * first paint) swaps in the document glyph INSIDE the same fixed box + * (no layout shift beyond the box, no broken-image placeholder). Text + * rows never get a box (the pre-phase bare-link cell, byte-identical). + * The glyph is static SVG (aria-hidden — the alt text is the + * accessible content); the box + img are properties only, never + * innerHTML with document-derived data (the house rule). */ import { fetchIsAdmin } from "./header.js"; @@ -1391,12 +1406,78 @@ export async function mount(root) { chunks: f.chunks, created_at: f.created_at, // phase 106 (task 08, D8): the tree's file date indexed_at: f.indexed_at, + // Phase 122 (task 04): the image affordance — the tree's + // file node carries is_image / image_url / summary on an + // image doc (the omission rule: a TEXT node's wire shape + // carries none, so these stay undefined there and makeRow + // keeps the bare-link cell, byte-identical to pre-phase). + is_image: f.is_image, + image_url: f.image_url, + summary: f.summary, }) ); } if (tableWrap) tableWrap.hidden = files.length === 0; } + /* Phase 122 (task 04): the document glyph — the fallback INSIDE the + * fixed thumbnail box (a failed fetch, or a node with no servable + * image_url). Static SVG, aria-hidden (the img's alt is the + * accessible content; this is decoration for the box). Built with + * createElementNS — the module keeps its ONE innerHTML (the static + * sync-modal skeleton, the test_kb_tree_ui pin). */ + function docThumbGlyph() { + const span = document.createElement("span"); + span.className = "kb-doc-thumb-glyph"; + span.setAttribute("aria-hidden", "true"); + const NS = "http://www.w3.org/2000/svg"; + const svg = document.createElementNS(NS, "svg"); + svg.setAttribute("viewBox", "0 0 48 48"); + svg.setAttribute("fill", "none"); + svg.setAttribute("stroke", "currentColor"); + svg.setAttribute("stroke-width", "2.4"); + svg.setAttribute("stroke-linecap", "round"); + svg.setAttribute("stroke-linejoin", "round"); + const sheet = document.createElementNS(NS, "path"); + sheet.setAttribute( + "d", + "M12 4h16l8 8v28a4 4 0 0 1-4 4H12a4 4 0 0 1-4-4V8a4 4 0 0 1 4-4Z" + ); + const fold = document.createElementNS(NS, "path"); + fold.setAttribute("d", "M28 4v8h8"); + svg.append(sheet, fold); + span.appendChild(svg); + return span; + } + + /* Phase 122 (task 04): the image-doc thumbnail — the FIXED 48px box + * (object-fit: cover via CSS, loading="lazy", alt = the summary — + * the vision description; a NULL/blank summary falls back to the + * title, then the path). A failed fetch swaps in the document + * glyph in the SAME box (progressive enhancement — no layout shift + * beyond the fixed box, no broken-image placeholder). Called only + * for image rows (makeRow gates on d.is_image). */ + function docThumb(d) { + const box = document.createElement("span"); + box.className = "kb-doc-thumb"; + const alt = + typeof d.summary === "string" && d.summary.trim() !== "" + ? d.summary + : d.title || d.path; + if (d.image_url) { + const img = document.createElement("img"); + img.className = "kb-doc-thumb-img"; + img.loading = "lazy"; + img.src = d.image_url; + img.alt = alt; + img.addEventListener("error", () => box.replaceChildren(docThumbGlyph())); + box.appendChild(img); + } else { + box.appendChild(docThumbGlyph()); + } + return box; + } + /* The no-data state (phase 97): zero sources (nothing registered, * nothing indexed) OR a failed tree fetch (the former showEmpty * failure behavior, unchanged in kind) — every catalog surface @@ -1440,7 +1521,18 @@ export async function mount(root) { }); link.title = d.path; // full path as the link's hover/accessible name link.textContent = d.path; - pathTd.appendChild(link); + // Phase 122 (task 04): an image row gets the FIXED 48px thumbnail + // box before the path link (the .kb-doc-path flex wrapper — the + // link keeps its ellipsis). Text rows keep the bare-link cell, + // byte-identical to pre-phase (no box at all). + if (d.is_image) { + const pathWrap = document.createElement("div"); + pathWrap.className = "kb-doc-path"; + pathWrap.append(docThumb(d), link); + pathTd.appendChild(pathWrap); + } else { + pathTd.appendChild(link); + } tr.appendChild(pathTd); // Phase 106 (task 08, D8): the cell order is [title, chunks, diff --git a/frontend/assets/styles.css b/frontend/assets/styles.css index d11a220..3e0f0c9 100644 --- a/frontend/assets/styles.css +++ b/frontend/assets/styles.css @@ -737,6 +737,47 @@ details.thinking .thinking-text ul { margin: 0 0 0.5rem; } } .source-chip:hover { background: var(--brand-soft); text-decoration: underline; } +/* Phase 122 (task 05): the sources block's COMPACT INLINE IMAGE for a + retrieved IMAGE document ("shown in the chat nicely", TODO L6) — the + .source-image figure sits beside its chip in the .msg-meta flex row + (additive: the chip's text and affordance stay). A capped (96px, + contain) img on the theme's surface, the document's summary as the + visible caption in the AA-safe muted ink (8.6:1 on --bg — the + row's page background). A failed image load removes the whole + figure in JS (the plain chip stays — never a broken-image icon). */ +.source-image { + display: inline-flex; + flex-direction: column; + align-items: center; + gap: 0.15rem; + max-width: 100%; + min-width: 0; + text-decoration: none; +} +.source-image-img { + display: block; + max-height: 96px; + max-width: 100%; + width: auto; + height: auto; + object-fit: contain; + background: var(--surface); + border: 1px solid var(--line); + border-radius: var(--radius-sm); +} +.source-image-caption { + max-width: 14rem; + color: var(--ink-soft); /* 8.6:1 on --bg (AA) — the row's page background */ + font-size: 0.7rem; + line-height: 1.25; + text-align: center; + display: -webkit-box; + -webkit-line-clamp: 2; + -webkit-box-orient: vertical; + overflow: hidden; +} +.source-image:hover { text-decoration: underline; } /* flat — the caption underlines, the chip pattern */ + /* Phase 113 (task 02): the related-docs row — the SECONDARY tier of scored docs (phase 113 task 01's usefulness bar demotes the sub-floor hits out of the citation surface; on a deflected turn the @@ -2246,6 +2287,61 @@ details.thinking .thinking-text ul { margin: 0 0 0.5rem; } text-decoration: underline; } +/* Image-document rows (phase 122, task 04): the file table's Path cell + carries a FIXED 48px thumbnail box before the path link (image rows + only — text rows keep the pre-phase bare-link cell, no box). The + .kb-doc-path flex wrapper gives the link its own ellipsis budget + (min-width: 0 — the .sync-label pattern) now that the box shares + the cell; object-fit: cover keeps every aspect ratio inside the + square (CSS on .kb-doc-thumb-img). The glyph fallback (a failed + fetch, or a node without a servable image_url) swaps INSIDE the + same fixed box — no layout shift beyond the box, no broken-image + placeholder. The img's alt (the vision description) carries the + accessibility; the glyph is aria-hidden. Phase-08 tokens only. */ +.kb-doc-path { + display: flex; + align-items: center; + gap: 0.5rem; + min-width: 0; + max-width: 100%; +} +.kb-doc-path .doc-link { + flex: 1 1 auto; + min-width: 0; /* lets the link shrink — what engages the ellipsis */ + overflow: hidden; + text-overflow: ellipsis; +} +.kb-doc-thumb { + flex: 0 0 auto; + display: block; + width: 48px; + height: 48px; + border: 1px solid var(--line); + border-radius: var(--radius-sm); + background: var(--bg); + color: var(--ink-soft); /* the glyph's stroke color (decorative) */ + overflow: hidden; +} +.kb-doc-thumb-img { + display: block; + width: 100%; + height: 100%; + object-fit: cover; +} +.kb-doc-thumb-glyph { + display: flex; + align-items: center; + justify-content: center; + width: 100%; + height: 100%; + padding: 9px; + box-sizing: border-box; +} +.kb-doc-thumb-glyph svg { + width: 100%; + height: 100%; +} + /* ---------- KB folder-description editor (phase 97, task 05) ---------- The phase-57 edit affordance on the RAG view's folder descriptions (the .kb-summary-* family, mirroring the viewer's .doc-summary-*): @@ -4267,6 +4363,44 @@ details.thinking .thinking-text ul { margin: 0 0 0.5rem; } line-height: 1.5; } +/* Image documents (phase 122, task 04): the viewer's image block — + the document's persistent bytes (the /api/documents/{id}/image + route) above its description (the .doc-raw content slot below). The + theme's surface card treatment (the .doc-md language): the image + sits on --surface with the line border + radius; max-width 100% + + max-height 70vh keep any aspect ratio inside the page (height: auto + keeps the ratio). Static content — no animation; the alt text (the + vision description) carries the accessibility (WCAG). Phase-08 + tokens only (no new hue — the monochrome invariant). */ +.doc-image { + width: 100%; + max-width: var(--chat-column); + margin: 0 auto 1rem; + display: flex; + justify-content: center; + background: var(--surface); + border: 1px solid var(--line); + border-radius: var(--radius); + box-shadow: var(--shadow); + padding: 0.5rem; +} +.doc-image-img { + display: block; + max-width: 100%; + max-height: 70vh; + height: auto; + border-radius: var(--radius-sm); + background: var(--bg); +} +/* The onerror fallback (the route's 404 — the row exists but the + copy was lost): a muted note in the card's place-of-image; the + description below still renders, so the doc stays readable. */ +.doc-image-unavailable { + margin: 1.25rem 1rem; + color: var(--ink-soft); /* 5.1:1 on --surface (AA) */ + font-size: 0.9rem; +} + /* Designed not-found state (no emoji — plain SVG mark, phase 08 rule). */ .doc-not-found { width: 100%; diff --git a/tests/conftest.py b/tests/conftest.py index ebff90a..1e09531 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -69,6 +69,15 @@ os.environ["BOR_RECENCY_HALF_LIFE_DAYS"] = str( _Settings.model_fields["recency_half_life_days"].default ) +# Phase 122 (task 01): the same leak class for the image-document +# toggle — an operator's local ``.env`` may legitimately carry +# ``BOR_IMAGES``/``BOR_IMAGE_EXTENSIONS``/``BOR_IMAGE_DIR``, and the +# "off by default" pins (the ``/api/config`` ``images`` flag, the +# walk's images-off behavior) must see the code defaults. +os.environ["BOR_IMAGES"] = str(_Settings.model_fields["images"].default) +os.environ["BOR_IMAGE_EXTENSIONS"] = str(_Settings.model_fields["image_extensions"].default) +os.environ["BOR_IMAGE_DIR"] = str(_Settings.model_fields["image_dir"].default) + from app.db import SessionLocal, db_available # noqa: E402 from app.main import app as fastapi_app # noqa: E402 diff --git a/tests/e2e/mock_llm.py b/tests/e2e/mock_llm.py index 82d68f1..06ac766 100644 --- a/tests/e2e/mock_llm.py +++ b/tests/e2e/mock_llm.py @@ -26,6 +26,14 @@ Implements just enough of the aipi surface: ``SUMMARY_MODE`` branch: the folder marker CONTAINS the summary marker as a substring, so the summary branch would otherwise shadow every folder-summary call + - ``IMAGE_DESCRIPTION_MODE`` (phase 122, image documents) -> the + fixed ``IMAGE_DESCRIPTION_ANSWER`` description: the CHAT model's + (vision) reply to ``summarizer.describe_image``'s multimodal user + message (the marker is its first text part; the call is + non-streaming, so only this answer path serves it). The reply is + the image document's whole content + summary, so it is byte- + stable and token-dense (its cosine against the story suite's + question clears the mock-calibrated gate) - ``DEFLECT_MODE`` -> honest "I haven't done anything like that" answer - otherwise -> upbeat answer quoting the provided document context - user message containing ``pretend to think slowly`` -> 3s warm-up delay @@ -592,19 +600,46 @@ def _messages(body: dict[str, Any]) -> list[dict[str, str]]: return body.get("messages", []) +def _content_text(content: Any) -> str: + """The TEXT of one message's content (phase 122 list safety). + + String content passes through byte-identical (as does an absent + value or an explicit ``None`` — the ``or ""`` semantics the + :func:`_context` docstring pins). A multimodal part LIST — the + phase-122 image description's ``[{type: "text", …}, + {type: "image_url", …}]``, the only list-content message the app + produces (``app.rag.summarizer.describe_image``) — contributes its + text parts joined (the ``image_url`` part carries no text): the + trigger checks and marker branches read the text, and no existing + string-content request is affected. + """ + if isinstance(content, list): + return " ".join( + part.get("text", "") + for part in content + if isinstance(part, dict) and part.get("type") == "text" + ) + return content or "" + + def _system(body: dict[str, Any]) -> str: - return " ".join(m.get("content", "") for m in _messages(body) if m.get("role") == "system") + return " ".join( + _content_text(m.get("content")) + for m in _messages(body) + if m.get("role") == "system" + ) def _user(body: dict[str, Any]) -> str: - parts = [m.get("content", "") for m in _messages(body) if m.get("role") == "user"] + parts = [_content_text(m.get("content")) for m in _messages(body) if m.get("role") == "user"] return parts[-1] if parts else "" def _context(body: dict[str, Any]) -> str: """The document context is the longest system/user message in practice. - ``m.get("content") or ""`` (NOT ``m.get("content", "")``): a well-formed + ``_content_text(m.get("content"))`` (the ``or ""`` semantics, NOT + ``m.get("content", "")``): a well-formed OpenAI tool-call message carries ``content: None`` EXPLICITLY (the app's agent loop appends exactly that — ``app/rag/agent.py``), and a forced final answer after a tool round (the round-cap path) reaches this helper @@ -612,9 +647,12 @@ def _context(body: dict[str, Any]) -> str: an explicit ``None`` and crashes ``len()`` with a 500 (phase 93 task 04 caught it via the deterministic single-read flow's ALREADY_IN_CONTEXT loop); ``or ""`` treats absent and explicit-None alike, so the fallback - composes deterministically instead of traceback-ing.""" + composes deterministically instead of traceback-ing. Phase 122: a + multimodal part list maps to its text parts (see + :func:`_content_text`), so ``len()``/slicing never meet a list. + """ msgs = _messages(body) - return max((m.get("content") or "" for m in msgs), key=len) + return max((_content_text(m.get("content")) for m in msgs), key=len) LONG_ANSWER_TRIGGER = "write a long answer" @@ -739,6 +777,25 @@ HISTORY_TRIGGER = "echo my history" #: phrase, so every other suite is unaffected. FOLDER_MAP_TRIGGER = "repeat your folder map" +#: Phase 122 (image documents, LOCKED A3): the fixed description the +#: mock's vision (CHAT) model returns for an ``IMAGE_DESCRIPTION_MODE`` +#: request — the multimodal user message +#: ``[{type: "text", …IMAGE_DESCRIPTION_MODE…}, {type: "image_url", +#: …}]`` (``app.rag.summarizer.describe_image``, the app's only +#: list-content message). It becomes the image document's WHOLE +#: ``content`` AND ``summary`` (the only embedded text — the embedding +#: model never sees pixels), so the text is byte-stable across runs and +#: token-dense: the story suite's question ("What is shown in the +#: homelab network diagram?", cosine ≈0.38 against the mock's +#: token-overlap embeddings) clears the mock-calibrated gate (0.30) +#: and the citation usefulness floor (0.15). +IMAGE_DESCRIPTION_ANSWER = ( + "A network diagram of the homelab server room: a core router on top, " + "a core switch in the middle, and three labeled subnets at the bottom — " + "VLAN 10 office, VLAN 20 lab, and VLAN 30 storage — with a legend of " + "cable runs. Title: Homelab Network Map." +) + TABLE_ANSWER = ( "Here's the shape, in a table:\n" "\n" @@ -2178,6 +2235,17 @@ def compose_answer(body: dict[str, Any]) -> str: answer = "Knowledge base outline:\n- " + " ".join( TOKEN_RE.findall(user.lower())[:8] ) + elif "IMAGE_DESCRIPTION_MODE" in user: + # Phase 122 (image documents, LOCKED A3): the CHAT model's + # (vision) image description — a NON-STREAMING multimodal user + # message, so only this path ever sees it. ``_content_text`` + # joins the text parts, so the marker (the first text part) + # lands in ``user``. Checked before the user-trigger branches: + # the marker is a fixed app constant, and a real question is + # never expected to type it (the other user triggers are owner + # phrasings a question might legitimately contain — this one + # is an app-to-app marker). + answer = IMAGE_DESCRIPTION_ANSWER elif TABLE_TRIGGER in user.lower(): # Markdown tables (phase 44, TODO.md L6): the story E2E's # deterministic table answer — a 3-column table, the diff --git a/tests/e2e/test_configurable_brand.py b/tests/e2e/test_configurable_brand.py index f501f2a..b27f287 100644 --- a/tests/e2e/test_configurable_brand.py +++ b/tests/e2e/test_configurable_brand.py @@ -149,27 +149,29 @@ def test_api_config_serves_both_names(testy_server: str, app_server: str) -> Non # "Save as doc" gating; both instances run with BOR_DOCS_REPO # empty, so it is the inert false here. Phase 62 (task 01): the # endpoint grew with the UI-customization keys; phase 91 - # (task 03) deleted the retired CSS-file theming's ``theme`` key — - # the five keys below are the entire contract (this suite's - # instances carry no UI-customization overrides, so the string - # keys are their defaults). + # (task 03) deleted the retired CSS-file theming's ``theme`` key; + # phase 122 (task 01) added the ``images`` flag (inert false here + # — neither instance sets BOR_IMAGES) — the six keys below are + # the entire contract (this suite's instances carry no + # UI-customization overrides, so the string keys are their + # defaults). assert set(body) == { "app_name", "version", "docs_repo_configured", - "input_placeholder", "footer_text", + "images", "input_placeholder", "footer_text", } assert body["app_name"] == TESTY_NAME assert body["docs_repo_configured"] is False + assert body["images"] is False # The shared conftest instance keeps the default (the other # suites' title/label contract rides on it) — and its key set - # tracks the endpoint contract (five keys after phase 91, - # task 03). + # tracks the endpoint contract (six keys after phase 122, task 01). r2 = httpx.get(f"{app_server}/api/config", timeout=5) assert r2.status_code == 200 r2_body = r2.json() assert set(r2_body) == { "app_name", "version", "docs_repo_configured", - "input_placeholder", "footer_text", + "images", "input_placeholder", "footer_text", } assert r2_body["app_name"] == DEFAULT_NAME diff --git a/tests/e2e/test_image_documents.py b/tests/e2e/test_image_documents.py new file mode 100644 index 0000000..b42e0ea --- /dev/null +++ b/tests/e2e/test_image_documents.py @@ -0,0 +1,499 @@ +"""Phase 122 E2E (Playwright): standalone image documents — the user path. + +Run in isolation (DB must be up: ``podman compose up -d db``): + + uv run pytest tests/e2e/test_image_documents.py -v --no-cov + +The suite's app instance runs with ``BOR_IMAGES=true`` (module env +override — the conftest per-suite-app pattern, leak guards included): +the standalone PNG arrives through the REAL admin upload flow (a zip +with a single image member — phase 90's no-scan contract: the upload +unpacks + registers, the RAG page's "Sync sources" button scans), the +mock's vision model (``IMAGE_DESCRIPTION_MODE`` branch — +``tests/e2e/mock_llm.py``) writes the description, and the document is +first-class at every surface: + +* the Sources page lists it with the 48px thumbnail (the image bytes + route, alt = the summary); +* the document viewer renders the image with the description below it + (the summary panel is suppressed — the summary IS the description); +* a grounded chat question shows the compact inline figure in the + answer's sources block (image + the summary caption, the "shown in + the chat nicely" contract). + +The negative case drives a SECOND module app with ``BOR_IMAGES`` +forced ``false`` (the DEFAULT contract — an operator's local ``.env`` +cannot leak the toggle in either direction: both apps pin the value +explicitly): the same upload + sync produces NO image document (the +walk is blind to the file, the source row stays a 0-document source). +""" +from __future__ import annotations + +import base64 +import json +import os +import re +import subprocess +import sys +import zipfile +from collections.abc import Iterator +from pathlib import Path + +import httpx +import pytest +from playwright.sync_api import Browser, Page, expect +from sqlalchemy import select, text + +from app.config import Settings +from app.db import SessionLocal +from app.models import Document +from app.rag.agent import IMAGE_DOC_MARKER +from e2e.auth_helpers import login +from e2e.conftest import ADMIN_PASSWORD, SESSION_SECRET, USE_REAL_LLM, _wait_http +from e2e.mock_llm import IMAGE_DESCRIPTION_ANSWER + +REPO = Path(__file__).resolve().parents[2] +GIT_SOURCES_URL = "/git-sources.html" +SOURCE_NAME = "e2e-image" # the archive stem (archive_source_name) +DOC_PATH = "pic.png" + +#: The grounded question, carrying the house scripted-read call +#: (``SUMMARY_SEED_READ_TRIGGER`` — the phase-119 A1 convention: a +#: zero-read grounded turn chips NOTHING, so the image doc must be +#: READ to earn its citation chip + figure): the mock emits the +#: scripted ``read e2e-image/pic.png``, then echoes the tool result. +#: Grounding: the mock's token-overlap cosine against the +#: ``IMAGE_DESCRIPTION_ANSWER`` description is ≈0.31 (over the +#: mock-calibrated gate (0.30), and the FTS leg corroborates — +#: homelab/network/diagram all hit — the 0.15 floor backstop). +IMAGE_QUESTION = ( + "Read the suggested document: read e2e-image/pic.png — " + "what is shown in the homelab network diagram?" +) + +#: A real 1×1 transparent PNG (the unit/integration suites' fixture — +#: the pipeline is content-agnostic, the well-formed bytes keep the +#: upload + serve + render path honest). +PNG_1X1 = base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJ" + "AAAAC0lEQVR4nGP4DwQACfsD/fteaysAAAAASUVORK5CYII=" +) + +UPLOAD_TIMEOUT_MS = 30_000 +SYNC_TIMEOUT_MS = 60_000 +SYNCED_LABEL = re.compile(r"^Synced \d{1,2}:\d{2}$") + + +# --------------------------------------------------------------------------- +# Module apps: images ON (the story) and images forced OFF (the default +# contract). Each owns its port + its scratch upload/source/image dirs. +# --------------------------------------------------------------------------- + + +def _app_env(mock_llm: int, app_port: int, *, images: bool, scratch: Path) -> dict[str, str]: + """The conftest app env (leak guards included) with the phase-122 + knobs: ``BOR_IMAGES`` pinned EXPLICITLY (true for the story app, + false for the default app — process env ranks above an operator's + local gitignored ``.env``, so the contract under test cannot leak + in either direction) and the image/upload homes in the suite's + scratch dir (the app under test must not write image copies into + the owner's real ``~/bor-sources``).""" + env = dict(os.environ) + env.pop("DEBUGPY", None) + env["BOR_ENVIRONMENT"] = "e2e" + env["BOR_STATIC_DIR"] = str(REPO / "frontend") + env["BOR_LLM_BASE_URL"] = ( + "https://aipi.reeseapps.com/v1" + if USE_REAL_LLM + else f"http://127.0.0.1:{mock_llm}/v1" + ) + # Mock-calibrated gate (conftest pattern) — the story's question + # grounds on these values (see IMAGE_QUESTION). + env["BOR_RELEVANCE_THRESHOLD"] = "0.30" + env["BOR_LEXICAL_SUPPORT_FLOOR"] = "0.15" + env["BOR_SOURCE_USEFULNESS_FLOOR"] = "0.15" + # Phase 67: instant retry waits + the code-default budget. + env["BOR_LLM_RETRY_DELAY"] = "0" + env["BOR_LLM_RETRIES"] = str(Settings.model_fields["llm_retries"].default) + env.setdefault( + "BOR_DATABASE_URL", + "postgresql+psycopg://reese:reese@localhost:5432/brain_of_reese", + ) + env["BOR_ADMIN_PASSWORD"] = ADMIN_PASSWORD + env["BOR_SESSION_SECRET"] = SESSION_SECRET + # Leak guards (conftest pattern). + env["BOR_GIT_SOURCES"] = "" + env["BOR_DOCS_REPO"] = "" + env["BOR_SUGGESTIONS"] = json.dumps( + Settings.model_fields["suggestions"].default + ) + env["BOR_INPUT_PLACEHOLDER"] = Settings.model_fields["input_placeholder"].default + env["BOR_FOOTER_TEXT"] = Settings.model_fields["footer_text"].default + # Phase 122: the toggle under test + the suite-private homes. + env["BOR_IMAGES"] = "true" if images else "false" + env["BOR_UPLOAD_DIR"] = str(scratch / "uploads") + env["BOR_SOURCES_DIR"] = str(scratch / "checkouts") + env["BOR_IMAGE_DIR"] = str(scratch / "images") + return env + + +@pytest.fixture(scope="module") +def app_server(mock_llm: int, tmp_path_factory: pytest.TempPathFactory) -> Iterator[str]: + """The story app — ``BOR_IMAGES=true`` (the module env override; + the conftest session app is never started in this isolated run, + so no port clash).""" + scratch = tmp_path_factory.mktemp("bor_image_on") + port = int(os.environ.get("E2E_APP_PORT_IMAGES", "8150")) + proc = subprocess.Popen( + [sys.executable, "-m", "uvicorn", "app.main:app", + "--host", "127.0.0.1", "--port", str(port), "--log-level", "warning"], + cwd=REPO, + env=_app_env(mock_llm, port, images=True, scratch=scratch), + ) + try: + _wait_http(f"http://127.0.0.1:{port}/api/health") + yield f"http://127.0.0.1:{port}" + finally: + proc.terminate() + try: + proc.wait(timeout=10) + except subprocess.TimeoutExpired: + proc.kill() + + +@pytest.fixture(scope="module") +def app_url(app_server: str) -> str: + return app_server + + +@pytest.fixture(scope="module") +def default_app_server( + mock_llm: int, tmp_path_factory: pytest.TempPathFactory +) -> Iterator[str]: + """The default-contract app — ``BOR_IMAGES=false`` (the LOCKED A3 + default; only the negative test starts it).""" + scratch = tmp_path_factory.mktemp("bor_image_off") + port = int(os.environ.get("E2E_APP_PORT_IMAGES_OFF", "8151")) + proc = subprocess.Popen( + [sys.executable, "-m", "uvicorn", "app.main:app", + "--host", "127.0.0.1", "--port", str(port), "--log-level", "warning"], + cwd=REPO, + env=_app_env(mock_llm, port, images=False, scratch=scratch), + ) + try: + _wait_http(f"http://127.0.0.1:{port}/api/health") + yield f"http://127.0.0.1:{port}" + finally: + proc.terminate() + try: + proc.wait(timeout=10) + except subprocess.TimeoutExpired: + proc.kill() + + +@pytest.fixture(scope="module") +def default_app_url(default_app_server: str) -> str: + return default_app_server + + +@pytest.fixture(scope="module") +def zip_path(tmp_path_factory: pytest.TempPathFactory) -> Path: + """The fixture archive: ONE standalone PNG at the root — source + ``e2e-image``, document path ``pic.png``.""" + root = tmp_path_factory.mktemp("bor_image_zip") + (root / DOC_PATH).write_bytes(PNG_1X1) + archive = root / f"{SOURCE_NAME}.zip" + with zipfile.ZipFile(archive, "w", zipfile.ZIP_DEFLATED) as zf: + zf.write(root / DOC_PATH, arcname=DOC_PATH) + return archive + + +# --------------------------------------------------------------------------- +# DB + UI helpers +# --------------------------------------------------------------------------- + + +def _truncate_all() -> None: + """Fresh KB + registry per test (the E2E isolation pattern): the + suites share one Postgres, and a leftover row would corrupt the + counts. ``sources_meta`` (the KB generation counter) resets with + the KB — the sync re-creates the row.""" + with SessionLocal() as db: + db.execute(text( + "TRUNCATE chunks, documents, query_log, kb_overview, " + "git_sources, sources_meta, folder_summaries" + )) + db.commit() + + +def _health_db(app_url: str) -> bool: + try: + return httpx.get(f"{app_url}/api/health", timeout=5).json()["db"] == "up" + except Exception: # noqa: BLE001 — unreachable is the skip case + return False + + +def _upload_zip(page: Page, app_url: str, archive: Path) -> None: + """The real admin upload flow: form login → the git-sources page → + pick the archive → submit → the terminal result line (phase 90: + "Uploaded — press Sync sources to import it.").""" + login(page, app_url, next=GIT_SOURCES_URL) + expect(page).to_have_url(app_url + GIT_SOURCES_URL, timeout=30_000) + expect(page.locator("#sign-out-btn")).to_be_visible(timeout=15_000) + page.set_input_files("#archive-upload-file", str(archive)) + page.click("#archive-upload-btn") + result = page.locator("#archive-upload-result") + expect(result).to_be_visible(timeout=UPLOAD_TIMEOUT_MS) + expect(result).to_have_text( + f"Uploaded {SOURCE_NAME} — press Sync sources to import it.", + timeout=UPLOAD_TIMEOUT_MS, + ) + + +def _sync_via_ui(page: Page, app_url: str, expected_result: str) -> None: + """The RAG page's "Sync sources" button (the scan — phase 90 A3): + click → "Syncing…" → the terminal "Synced HH:MM" label + the fresh + counts in #sync-result (the never-stale contract, test_git_source_ + dates' lifecycle wait).""" + page.goto(app_url + "/sources.html") + btn = page.locator("#sync-btn") + expect(btn).to_be_visible(timeout=30_000) + expect(page.locator("#sync-label")).to_have_text( + re.compile(r"^Sync sources$|^Synced \d{1,2}:\d{2}$") + ) + btn.click() + expect(btn).to_be_disabled() + expect(page.locator("#sync-label")).to_have_text("Syncing…") + expect(page.locator("#sync-label")).to_have_text(SYNCED_LABEL, timeout=SYNC_TIMEOUT_MS) + expect(btn).to_be_enabled() + expect(page.locator("#sync-result")).to_have_text(expected_result) + + +def _drill_to_source(page: Page, name: str) -> None: + """Top level → the source (the file table then holds its direct + files — ours is at the source root).""" + page.locator("#folders-tbody .folder-link").first.wait_for(state="visible") + page.click(f'#folders-tbody a.folder-link:text-is("{name}")') + expect(page.locator("#docs-tbody tr").first).to_be_visible(timeout=30_000) + + +# --------------------------------------------------------------------------- +# The module seed: upload + sync the fixture image through the real UI +# (the story E2E truncate/re-import-per-module pattern). +# --------------------------------------------------------------------------- + + +@pytest.fixture(scope="module") +def seeded( + app_url: str, + browser: Browser, + zip_path: Path, +) -> Iterator[dict[str, str]]: + """Upload the single-PNG zip through the admin UI, then scan it + with the RAG page's Sync button (the mock's vision model describes + the image in-process). Yields the seeded doc's identity (the bytes + route + the description) for the render assertions. The module + app's ``/api/config`` flag is asserted first — the env override + must have reached the app under test, or the whole premise of the + suite is void.""" + if not _health_db(app_url): + pytest.skip("Postgres not reachable — run `podman compose up -d db` first") + cfg = httpx.get(f"{app_url}/api/config", timeout=5).json() + assert cfg["images"] is True, "the module app must run with BOR_IMAGES=true" + + _truncate_all() + page = browser.new_page(viewport={"width": 1280, "height": 800}) + try: + _upload_zip(page, app_url, zip_path) + _sync_via_ui(page, app_url, expected_result="1 added") + with SessionLocal() as db: + doc = db.scalar( + select(Document).where( + Document.source == SOURCE_NAME, Document.path == DOC_PATH + ) + ) + assert doc is not None, "the uploaded image must become a document" + assert doc.is_image is True and doc.image_path is not None + # content == summary == the mock's description (the ONLY + # embedded text of the doc — the pipeline contract, LOCKED A3). + assert doc.content == IMAGE_DESCRIPTION_ANSWER + assert doc.summary == IMAGE_DESCRIPTION_ANSWER + finally: + page.close() + yield { + "image_url": f"/api/documents/{doc.id}/image", + "description": IMAGE_DESCRIPTION_ANSWER, + } + _truncate_all() + + +# --------------------------------------------------------------------------- +# 1. Sources page: the image doc is listed with its thumbnail +# --------------------------------------------------------------------------- + + +def test_sources_page_lists_the_image_doc( + page: Page, app_url: str, seeded: dict[str, str] +) -> None: + """The KB total is the image doc (1 doc, 2 chunks — the one + content chunk + the phase-30 ``is_summary`` chunk), the drill-down + tree reaches it, and its row carries the FIXED 48px thumbnail box: + a lazy ```` from the bytes route (not the glyph fallback — + the fixture PNG is served), ``alt`` = the summary (the WCAG + contract).""" + login(page, app_url) # lands on /sources.html + expect(page.locator("#stat-docs")).to_have_text("1") + expect(page.locator("#stat-chunks")).to_have_text("2") + expect(page.locator("#sources-empty")).to_be_hidden() + + _drill_to_source(page, SOURCE_NAME) + row = page.locator("#docs-tbody tr", has_text=DOC_PATH) + expect(row).to_have_count(1) + + box = row.locator(".kb-doc-thumb") + expect(box).to_be_visible() + img = row.locator(".kb-doc-thumb-img") + expect(img).to_have_count(1) + expect(img).to_be_visible(timeout=15_000) # the fetch resolves — no glyph + expect(img).to_have_attribute("src", seeded["image_url"]) + expect(img).to_have_attribute("loading", "lazy") + expect(img).to_have_attribute("alt", seeded["description"]) + expect(row.locator(".kb-doc-thumb-glyph")).to_have_count(0) + + +# --------------------------------------------------------------------------- +# 2. Document viewer: the image renders, the description below it +# --------------------------------------------------------------------------- + + +def test_document_viewer_renders_image_and_description( + page: Page, app_url: str, seeded: dict[str, str] +) -> None: + """The Sources row's path link opens the same-page document modal + (phase 26): the ``is_image`` content block renders the PERSISTENT + bytes first (``.doc-image-img`` from the bytes route, ``alt`` = + the summary), and the description follows in the normal content + slot (``pre.doc-raw`` — the plain-content path). The labeled + Summary panel is SUPPRESSED for the verbatim-description case + (summary === content — the importer invariant; the panel would + duplicate the text right below the image).""" + login(page, app_url) + _drill_to_source(page, SOURCE_NAME) + page.locator(f'#docs-tbody a.doc-link:text-is("{DOC_PATH}")').click() + + modal = page.locator("#doc-modal") + expect(modal).to_be_visible(timeout=30_000) + expect(page.locator("#doc-modal-title")).to_have_text("pic") + + img = modal.locator(".doc-image-img") + expect(img).to_have_count(1) + expect(img).to_be_visible(timeout=15_000) # the bytes route serves the PNG + expect(img).to_have_attribute("src", seeded["image_url"]) + expect(img).to_have_attribute("alt", seeded["description"]) + + # The description below the image (the doc's readable content IS + # the vision description). + expect(modal.locator("pre.doc-raw")).to_have_text(seeded["description"]) + # The verbatim case: no duplicate Summary panel. + expect(modal.locator(".doc-summary")).to_have_count(0) + # No "Image unavailable" note — the copy is intact. + expect(modal.locator(".doc-image-unavailable")).to_have_count(0) + + +# --------------------------------------------------------------------------- +# 3. Chat: a grounded question shows the inline image in the sources +# --------------------------------------------------------------------------- + + +def test_chat_sources_block_shows_the_inline_image( + page: Page, app_url: str, seeded: dict[str, str] +) -> None: + """A question the mock grounds on the image doc (cosine ≈0.31, + FTS-corroborated), scripted to READ it (phase 119 A1 — a zero-read + turn chips nothing): the mock's echo carries the agent ``read`` + result VERBATIM — including the task-05 ``IMAGE_DOC_MARKER`` line + (the model saw the description, not raw bytes) — and the answer's + sources block carries the citation chip (the READ doc clears the + usefulness floor) AND the compact inline figure — the ```` + from the bytes route, the visible caption + ``alt`` settling to + the document's summary (the figure fetches it from the content + endpoint — the frame carries no summary), the "shown in the chat + nicely" contract.""" + login(page, app_url, next="/") + expect(page.locator("#kb-banner")).to_be_hidden() + + page.fill("#message-input", IMAGE_QUESTION) + page.click("#send-btn") + + bubble = page.locator(".msg.brain .bubble").last + # The scripted read's echoed result — the marker line (task 05) + # and the description itself (the model's view of the image). + expect(bubble).to_contain_text(IMAGE_DOC_MARKER, timeout=30_000) + expect(bubble).to_contain_text(seeded["description"], timeout=30_000) + expect(page.locator("#send-btn")).to_be_enabled(timeout=30_000) + expect(page.locator("#send-label")).to_have_text("Send") + + # The citation chip (the text affordance stays — the figure is + # additive, not a replacement). + chip = page.locator(".msg.brain .source-chip") + expect(chip).to_have_count(1) + expect(chip).to_have_text(f"{SOURCE_NAME}/{DOC_PATH}") + + # The inline figure: image + caption, both settling to the summary + # (the content fetch resolves the alt/caption after the title). + fig = page.locator(".msg.brain .source-image") + expect(fig).to_have_count(1) + img = fig.locator(".source-image-img") + expect(img).to_be_visible(timeout=15_000) + expect(img).to_have_attribute("src", seeded["image_url"]) + expect(img).to_have_attribute("alt", seeded["description"], timeout=15_000) + caption = fig.locator(".source-image-caption") + expect(caption).to_have_text(seeded["description"], timeout=15_000) + + +# --------------------------------------------------------------------------- +# 4. The default contract: BOR_IMAGES off (the default) → the same +# upload + sync produces NO image document +# --------------------------------------------------------------------------- + + +def test_default_env_upload_produces_no_image_doc( + page: Page, default_app_url: str, zip_path: Path +) -> None: + """The off-by-default contract end-to-end: with ``BOR_IMAGES`` + false, the SAME upload + sync is byte-identical to the pre-phase + walk — the PNG is invisible to the scan (0 files, 0 added), the + source row stays a registered 0-document source, and NO documents + row (let alone an ``is_image`` one) exists. The file still lands + on disk (the unpack is unchanged — only the walk filter changes).""" + if not _health_db(default_app_url): + pytest.skip("Postgres not reachable — run `podman compose up -d db` first") + cfg = httpx.get(f"{default_app_url}/api/config", timeout=5).json() + assert cfg["images"] is False, "the default app must run with images off" + + _truncate_all() + try: + _upload_zip(page, default_app_url, zip_path) + _sync_via_ui( + page, default_app_url, expected_result="0 added · 0 unchanged" + ) + + # No document at all — the image file is not even a "file" to + # the images-off walk (not unknown, not indexed). + with SessionLocal() as db: + assert ( + db.scalar(select(Document).where(Document.is_image.is_(True))) + is None + ) + assert db.scalar(select(Document).where(Document.source == SOURCE_NAME)) is None + + # The Sources page: 0 docs, the source row (registered) drills + # to an EMPTY file table. + page.goto(default_app_url + "/sources.html") + expect(page.locator("#stat-docs")).to_have_text("0") + expect(page.locator("#stat-chunks")).to_have_text("0") + page.locator("#folders-tbody .folder-link").first.wait_for(state="visible") + page.click(f'#folders-tbody a.folder-link:text-is("{SOURCE_NAME}")') + expect(page.locator("#docs-tbody tr")).to_have_count(0) + finally: + _truncate_all() diff --git a/tests/e2e/test_ui_customization.py b/tests/e2e/test_ui_customization.py index 4c03ba0..629d771 100644 --- a/tests/e2e/test_ui_customization.py +++ b/tests/e2e/test_ui_customization.py @@ -164,14 +164,15 @@ def test_config_serves_the_overrides(custom_server: str) -> None: r = httpx.get(f"{CUSTOM_URL}/api/config", timeout=5) assert r.status_code == 200 body = r.json() - # The five-key set (the phase-39/59/62 endpoint contract, phase 91 - # task 03: the retired CSS-file theming's ``theme`` key is gone) - # with the two customization overrides — the app NAME stays the - # default (this suite does not re-test BOR_APP_NAME; that is the - # phase-39 suite's job). + # The six-key set (the phase-39/59/62 endpoint contract, phase 91 + # task 03: the retired CSS-file theming's ``theme`` key is gone; + # phase 122 task 01: the ``images`` flag) with the two + # customization overrides — the app NAME stays the default (this + # suite does not re-test BOR_APP_NAME; that is the phase-39 + # suite's job). assert set(body) == { "app_name", "version", "docs_repo_configured", - "input_placeholder", "footer_text", + "images", "input_placeholder", "footer_text", } assert body["app_name"] == DEFAULT_NAME assert body["input_placeholder"] == CUSTOM_PLACEHOLDER diff --git a/tests/fakes.py b/tests/fakes.py index 5883202..7f89bbf 100644 --- a/tests/fakes.py +++ b/tests/fakes.py @@ -1,6 +1,8 @@ """Shared test fakes (no network, deterministic).""" from __future__ import annotations +from typing import Any + from app.config import Settings from app.rag.llm import LLMError @@ -15,6 +17,11 @@ class FakeEmbedder: ``"Summary of "`` and raises :class:`LLMError` when the content contains the sentinel word ``SUMMARY-BLOWUP`` (drives the importer's fail-soft summary path). + ``content`` may be a string or a phase-122 multimodal part list + (``dict[str, Any]`` messages, the ``LLMClient.chat`` shape) — the + text-summary body above is string-only; a subclass handling the + multimodal image description (task 03's vision mock) overrides + ``chat``. """ def __init__(self, dim: int = 768) -> None: @@ -36,7 +43,7 @@ class FakeEmbedder: return vec async def chat( - self, messages: list[dict[str, str]], model: str | None = None + self, messages: list[dict[str, Any]], model: str | None = None ) -> str: self.chat_calls.append(list(messages)) user = next((m["content"] for m in messages if m.get("role") == "user"), "") diff --git a/tests/integration/test_api.py b/tests/integration/test_api.py index 9f7fb4e..444e0b7 100644 --- a/tests/integration/test_api.py +++ b/tests/integration/test_api.py @@ -48,25 +48,30 @@ def test_health_reports_ok(client) -> None: def test_config_returns_default_app_metadata(client, db: Session) -> None: - """GET /api/config is public (anonymous) and returns exactly five + """GET /api/config is public (anonymous) and returns exactly six keys — the phase-39 app metadata, the phase-59 docs flag (inert false while BOR_DOCS_REPO is empty — the "Save as doc" gating), - and the phase-62 UI customization strings (composer placeholder, - footer line). Phase 91: with an empty ui_settings table the - effective strings are the env defaults (B1 — DB-over-env, the row - absent here); the retired CSS-file theming's ``theme`` key is gone - (task 03 — the five keys are the entire contract).""" + the phase-122 images flag (default false in the test env — + LOCKED A3: off by default), and the phase-62 UI customization + strings (composer placeholder, footer line). Phase 91: with an + empty ui_settings table the effective strings are the env defaults + (B1 — DB-over-env, the row absent here); the retired CSS-file + theming's ``theme`` key is gone (task 03 — the six keys are the + entire contract).""" _clear_ui_settings(db) r = client.get("/api/config") assert r.status_code == 200 body = r.json() assert set(body) == { "app_name", "version", "docs_repo_configured", - "input_placeholder", "footer_text", + "images", "input_placeholder", "footer_text", } assert body["app_name"] == "Brain of Reese" assert body["version"] == get_settings().app_version assert body["docs_repo_configured"] is False + # Phase 122 (task 01): the images flag is the default off (the + # test env sets no BOR_IMAGES) — a real bool, not a truthy string. + assert body["images"] is False # Phase 62: UNSET => the phase-61 neutral copy stands (the # byte-identical contract). assert body["input_placeholder"] == "Ask me anything…" @@ -91,7 +96,7 @@ def test_config_follows_overridden_app_name(client, db: Session) -> None: body = r.json() assert set(body) == { "app_name", "version", "docs_repo_configured", - "input_placeholder", "footer_text", + "images", "input_placeholder", "footer_text", } assert body["app_name"] == "Brain of Testy" assert body["version"] == "0.1.0" @@ -122,7 +127,7 @@ def test_config_serves_ui_customization_overrides(client, db: Session) -> None: body = r.json() assert set(body) == { "app_name", "version", "docs_repo_configured", - "input_placeholder", "footer_text", + "images", "input_placeholder", "footer_text", } assert body["input_placeholder"] == "Ask the vault…" assert body["footer_text"] == "Powered by my own models" @@ -153,6 +158,29 @@ def test_config_docs_flag_tracks_settings(client, db: Session) -> None: fastapi_app.dependency_overrides.clear() +def test_config_images_flag_tracks_settings(client, db: Session) -> None: + """Phase 122 (task 01): ``images`` mirrors ``settings.images`` + (``BOR_IMAGES``) — a real bool (never a truthy string) that flips + true the moment the toggle is on: that flag is the entire frontend + gating of the image affordances (the phase-123 attach control, + optionally the Sources page hint).""" + from app.config import Settings + from app.main import app as fastapi_app + + _clear_ui_settings(db) + fastapi_app.dependency_overrides[get_settings] = lambda: Settings( + images=True, + ) + try: + r = client.get("/api/config") + assert r.status_code == 200 + body = r.json() + assert isinstance(body["images"], bool) + assert body["images"] is True + finally: + fastapi_app.dependency_overrides.clear() + + def test_suggestions_returns_list(client) -> None: # Phase 79: the chips are user-gated — sign in as the admin first # (the test's purpose is the list shape, not the auth contract). diff --git a/tests/integration/test_auth_api.py b/tests/integration/test_auth_api.py index 2b6dde7..ebd79d5 100644 --- a/tests/integration/test_auth_api.py +++ b/tests/integration/test_auth_api.py @@ -431,8 +431,11 @@ def test_document_content_admin_contract(client: TestClient, db) -> None: "content", "indexed_at", "chunks", + "is_image", # added in phase 122 (task 04) — always present } assert body["summary"] is None + assert body["is_image"] is False # text doc — and no image_url key (never null) + assert "image_url" not in body # Unknown docs still 404 (same shape as the phase-16 pin). r = client.get("/api/documents/content", params={"source": "docs", "path": "nope.md"}) diff --git a/tests/integration/test_chat_api.py b/tests/integration/test_chat_api.py index 8f05b64..d2feaaa 100644 --- a/tests/integration/test_chat_api.py +++ b/tests/integration/test_chat_api.py @@ -2076,3 +2076,124 @@ def test_done_event_related_defaults_empty_and_old_payload_parses() -> None: dumped = ChatDoneEvent(**new_payload).model_dump() assert [r["path"] for r in dumped["related"]] == ["b.md"] assert [r["path"] for r in dumped["sources"]] == ["a.md"] + + +# --------------------------------------------------------------------------- +# Phase 122, task 05 — the SSE source frame's OPTIONAL image_url (the +# shared ``source_ref_with_image`` builder: present on an image doc's +# ref only, omitted — never null — on every text ref). Task 06 +# finalizes the phase-122 suite here with the story-level pins. +# --------------------------------------------------------------------------- + + +def _seed_image_doc(db) -> Document: + """An ``is_image`` documents row the agent can ``read`` (no chunks + needed — the read tool resolves the row by ``(source, path)`` and + serves its ``content``; the frame needs ``is_image`` + ``id`` + only). The fixture's teardown truncates the tables.""" + doc = Document( + id=uuid.uuid4(), + source="docs", + path="pic.png", + full_path="/tmp/pic.png", + title="pic", + content="A red square on a white background.", + summary="A red square on a white background.", + content_hash="0" * 64, + created_at=_FIXTURE_CREATED_AT, + is_image=True, + image_path="/tmp/pic.png", + ) + db.add(doc) + db.commit() + return doc + + +def test_grounded_turn_reading_image_doc_frame_carries_image_url_only_for_it( + client, db, seeded_kb: FakeRagLLM +) -> None: + """Task 05 wire contract: a mocked grounded answer whose agent + READS an image doc → the done frame's ref for THAT doc alone + carries ``image_url`` (the bytes route the frontend's sources + block renders from); the text-doc related refs carry NO + ``image_url`` key at all (the omission rule — the key is absent, + never null).""" + doc = _seed_image_doc(db) + scripted = FakeRagLLM( + tool_script=[ + [ + ToolCallPiece( + id="call_1", + name="read", + arguments={"path": "docs/pic.png"}, + ) + ] + ] + ) + fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: scripted + try: + _, _, frames = _stream_chat(client, QUESTION) + finally: + fastapi_app.dependency_overrides.clear() + + done = frames[-1] + assert done["deflected"] is False + # The read image doc is the citation surface (phase 119, A1) — and + # its ref is the frame's ONLY image_url carrier. + sources = done["sources"] + assert [(s["source"], s["path"]) for s in sources] == [("docs", "pic.png")] + assert sources[0]["image_url"] == f"/api/documents/{doc.id}/image" + # The related tier (ranks 6–7 for the Kubernetes question) is text + # docs — the key is ABSENT on every one of them, not null. + related = done["related"] + assert related + for ref in related: + assert "image_url" not in ref + + +def test_text_only_grounded_turn_frame_has_no_image_url_key_anywhere( + client, db, seeded_kb: FakeRagLLM +) -> None: + """The omission rule at the BYTE level (the phase's byte-identity + criterion): a grounded turn whose cited + related docs are ALL + text docs serializes a done frame with no ``image_url`` key + anywhere — checked on the raw wire text (not a re-serialized + dict), and every ref keeps exactly the pre-phase-122 key set.""" + scripted = FakeRagLLM( + tool_script=[ + [ + ToolCallPiece( + id="call_1", + name="read", + arguments={"path": "docs/homelab/kubernetes.md"}, + ) + ] + ] + ) + fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: scripted + try: + with client.stream("POST", "/api/chat", json={"message": QUESTION}) as r: + assert r.status_code == 200 + buf = "" + raw_frames: list[str] = [] + for part in r.iter_text(): + buf += part + while "\n\n" in buf: + frame, buf = buf.split("\n\n", 1) + frame = frame.strip() + if frame.startswith("data:"): + raw_frames.append(frame) + finally: + fastapi_app.dependency_overrides.clear() + + done_raw = [f for f in raw_frames if '"type": "done"' in f] + assert len(done_raw) == 1 + # The BYTE check: the key is absent from the wire text itself. + assert "image_url" not in done_raw[0] + done = json.loads(done_raw[0].removeprefix("data:").strip()) + assert done["deflected"] is False + assert [(s["source"], s["path"]) for s in done["sources"]] == [ + ("docs", "homelab/kubernetes.md") + ] + for ref in done["sources"] + done["related"]: + assert set(ref) == {"source", "path", "title"} # the pre-122 key set diff --git a/tests/integration/test_docs_api.py b/tests/integration/test_docs_api.py index efbaf16..6884976 100644 --- a/tests/integration/test_docs_api.py +++ b/tests/integration/test_docs_api.py @@ -7,22 +7,33 @@ Uses the real compose Postgres (``db`` fixture) and FastAPI's TestClient. """ from __future__ import annotations +import asyncio +import base64 +import hashlib import inspect import itertools +import logging import uuid from datetime import UTC, datetime, timedelta +from pathlib import Path +from typing import Any import pytest from fastapi.testclient import TestClient from sqlalchemy import delete, func, select, text import app.api.docs as docs_api +import app.rag.importer as rag_importer from app.config import Settings from app.core import tokens as token_service from app.main import app as fastapi_app from app.models import Chunk, Document, FolderSummary, GitSource from app.rag import git_sources as rag_git_sources from app.rag.folder_summaries import missing_folder_summaries +from app.rag.importer import import_sources +from app.rag.llm import LLMError +from app.rag.summarizer import DESCRIBE_PROMPT +from tests.fakes import FakeEmbedder _TREE_TABLES = "chunks, documents, folder_summaries, git_sources" @@ -823,3 +834,603 @@ def test_docs_tree_stat_walk_equivalence_with_flat_list(admin_client, db) -> Non ) _truncate_tree_tables(db) + +# --------------------------------------------------------------------------- +# Phase 122, task 02 — image ingest end-to-end (the full ``import_sources`` +# pipeline against the real DB; task 06 finalizes the phase-122 suite here +# with the image route + content-endpoint shapes). +# --------------------------------------------------------------------------- + +#: A real 1×1 transparent PNG — the importer is content-agnostic (it +#: never parses the image), but a well-formed fixture keeps the tests +#: honest about what a real upload looks like. +PNG_1X1 = base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAC0lEQVR4nGP4DwQACfsD/fteaysAAAAASUVORK5CYII=" +) + + +def _image_llm[ + ImageLLM: FakeEmbedder +](tmp_path, llm_cls: type[ImageLLM] = FakeEmbedder, **kwargs) -> ImageLLM: + """A fake LLM with the phase-122 image knobs (toggle ON by default; + the image dir defaults under *tmp_path* unless overridden). + *llm_cls* (task 03) may be the mock vision client (``_MockVisionLLM`` + below) for the no-seam-patch end-to-end path — the PEP 695 type + parameter keeps the helper's return type honest (``chat_models``). + """ + kwargs.setdefault("_env_file", None) + kwargs.setdefault("images", True) + kwargs.setdefault("image_dir", str(tmp_path / "images")) + llm = llm_cls() + llm.settings = Settings(**kwargs) # pyright: ignore[reportCallIssue] + return llm + + +def _patch_description(monkeypatch, description) -> None: + """Pin the task-02 seam (``rag_importer._describe_or_skip``) to + *description*. Task 03 fills the seam with the CHAT model's vision + call — these end-to-end mechanics do not change with it.""" + + async def _fake(llm, *, data, source, rel, full_path): + return description + + monkeypatch.setattr(rag_importer, "_describe_or_skip", _fake) + + +def _cleanup_source(db, source: str) -> None: + for doc in db.scalars(select(Document).where(Document.source == source)).all(): + db.delete(doc) + db.commit() + + +def test_import_sources_images_on_indexes_image_docs( + db, tmp_path, monkeypatch: pytest.MonkeyPatch +) -> None: + """``images=True`` end-to-end: a standalone image in a source becomes + a Document — bytes digested, the persistent copy in ``image_dir`` + (``.png``), ``content`` = the (mock) description, and ONLY + that text is embedded (the content chunks + the phase-30 summary + chunk, 768 dims) — while a text doc in the same source stays a + plain text doc.""" + source_root = tmp_path / "imgsource" + source_root.mkdir() + (source_root / "diagram.png").write_bytes(PNG_1X1) + (source_root / "notes.md").write_text("# Notes\n\nBody text.\n", encoding="utf-8") + llm = _image_llm(tmp_path) + _patch_description(monkeypatch, "A network diagram of the homelab VLANs.") + try: + summary = asyncio.run( + import_sources([source_root], llm, session=db, prune=True) + ) + assert (summary.files, summary.added, summary.images_failed) == (2, 2, 0) + assert summary.formats == {"md": 1, "png": 1} + + doc = db.scalar( + select(Document).where( + Document.source == source_root.name, Document.path == "diagram.png" + ) + ) + assert doc is not None, "the image file must become a document" + assert doc.is_image is True and doc.image_path is not None + assert doc.content == "A network diagram of the homelab VLANs." + assert doc.title == "diagram" # the non-markdown stem rule + copy = Path(doc.image_path) + assert copy.parent == Path(llm.settings.image_dir) + assert copy.name == f"{doc.id}.png" + assert copy.read_bytes() == PNG_1X1 + + # The ONLY embedded text is the description (the embedding model + # never sees pixels): every content chunk carries it, the + # phase-30 summary chunk exists + is embedded, 768 dims. Task + # 03: the description IS the summary (stored verbatim — no + # ``lite`` call, no pointer line), so the summary chunk mirrors + # ``doc.content`` exactly. + chunks = db.scalars(select(Chunk).where(Chunk.document_id == doc.id)).all() + content_chunks = [c for c in chunks if not c.is_summary] + assert [c.content for c in content_chunks] == [doc.content] + assert doc.summary == doc.content + summary_chunks = [c for c in chunks if c.is_summary] + assert len(summary_chunks) == 1 and summary_chunks[0].position == -1 + assert summary_chunks[0].content == doc.content + for c in chunks: + assert c.embedding is not None and len(c.embedding) == 768 + + # The text doc is untouched by the image machinery. + md = db.scalar( + select(Document).where( + Document.source == source_root.name, Document.path == "notes.md" + ) + ) + assert md is not None and md.is_image is False and md.image_path is None + finally: + _cleanup_source(db, source_root.name) + + +class _MockVisionLLM(FakeEmbedder): + """The phase-122 mock VISION client (task 03): the CHAT model + answers the multimodal describe call (the image bytes' data URL) + with a fixed, retrieval-oriented description; text (``lite``) calls + keep the ``FakeEmbedder`` behaviour. The REAL + ``rag_importer._describe_or_skip`` → ``summarizer.describe_image`` + chain runs end-to-end against it (no seam patch); every chat + call's model is recorded (``chat_models``).""" + + DESCRIPTION = ( + "A network diagram of the homelab VLANs: the core switch, the " + "router, and three labeled subnets." + ) + + def __init__(self) -> None: + super().__init__() + self.chat_models: list[str | None] = [] + + async def chat(self, messages, model=None): + self.chat_calls.append(list(messages)) + self.chat_models.append(model) + user = next((m["content"] for m in messages if m.get("role") == "user"), "") + if isinstance(user, list): + # The phase-122 describe call — the multimodal message. + return self.DESCRIPTION + first = user.split() + return "Summary of " + (first[0] if first else "") + + +def test_import_sources_mock_vision_client_end_to_end(db, tmp_path) -> None: + """Task 03 end-to-end (NO seam patch): a fixture PNG through the + mock vision client — the real ``_describe_or_skip`` → + ``describe_image`` → CHAT-model call — yields a doc whose + ``content`` == ``summary`` == the description, with its + ``is_summary`` chunk embedded, and whose ONLY embedded text is that + description (the embedding model never sees pixels).""" + source_root = tmp_path / "visione2e" + source_root.mkdir() + (source_root / "diagram.png").write_bytes(PNG_1X1) + llm = _image_llm(tmp_path, _MockVisionLLM) + try: + summary = asyncio.run( + import_sources([source_root], llm, session=db) + ) + assert (summary.files, summary.added, summary.images_failed) == (1, 1, 0) + # The describe call went to the CHAT model (LOCKED A3), once. + assert llm.chat_models == [llm.settings.llm_chat_model] + + # The wire shape: the multimodal user message — the fixed + # prompt's text part + the image's data-URL part. + (message,) = llm.chat_calls[0] + assert message["role"] == "user" + content: Any = message["content"] # the multimodal part list + assert content[0] == {"type": "text", "text": DESCRIBE_PROMPT} + assert content[1]["type"] == "image_url" + assert content[1]["image_url"]["url"].startswith("data:image/png;base64,") + + doc = db.scalar( + select(Document).where( + Document.source == source_root.name, Document.path == "diagram.png" + ) + ) + assert doc is not None, "the image file must become a document" + assert doc.is_image is True and doc.image_path is not None + assert doc.content == _MockVisionLLM.DESCRIPTION + assert doc.summary == _MockVisionLLM.DESCRIPTION # task 03: verbatim + + # The ONLY embedded text of the doc is the description — twice + # (the one content chunk + the is_summary chunk), 768 dims. + chunks = db.scalars(select(Chunk).where(Chunk.document_id == doc.id)).all() + summary_chunks = [c for c in chunks if c.is_summary] + assert len(summary_chunks) == 1 and summary_chunks[0].position == -1 + assert all(c.content == _MockVisionLLM.DESCRIPTION for c in chunks) + for c in chunks: + assert c.embedding is not None and len(c.embedding) == 768 + embedded = [t for batch in llm.calls for t in batch] + assert embedded == [_MockVisionLLM.DESCRIPTION] * 2 + finally: + _cleanup_source(db, source_root.name) + + +class _MockVisionFailsLLM(FakeEmbedder): + """A NON-VISION chat model (LOCKED A3's honest failure): the + multimodal describe call raises (the SDK errors — a chat model + without vision rejects the ``image_url`` part), text (``lite``) + calls keep the ``FakeEmbedder`` behaviour.""" + + async def chat(self, messages, model=None): + self.chat_calls.append(list(messages)) + user = next((m["content"] for m in messages if m.get("role") == "user"), "") + if isinstance(user, list): + raise LLMError("simulated non-vision chat model (test sentinel)") + first = user.split() + return "Summary of " + (first[0] if first else "") + + +def test_import_sources_failing_vision_skips_image_keeps_sync_green( + db, tmp_path, caplog: pytest.LogCaptureFixture +) -> None: + """LOCKED A3 fail-soft end-to-end (the task-06 integration pin): + a fixture PNG through a NON-VISION chat model — the real seam, no + patch — skips the image doc (``images_failed == 1``, NO row, NO + orphan copy — not even the image dir) while the TEXT doc in the + same source is indexed as usual (the sync completes, no row + mutation anywhere for the failed image).""" + source_root = tmp_path / "visionfail" + source_root.mkdir() + (source_root / "diagram.png").write_bytes(PNG_1X1) + (source_root / "notes.md").write_text("# Notes\n\nBody.\n", encoding="utf-8") + llm = _image_llm(tmp_path, _MockVisionFailsLLM) + try: + with caplog.at_level(logging.INFO, logger="app.importer"): + summary = asyncio.run( + import_sources([source_root], llm, session=db) + ) + assert (summary.files, summary.added, summary.images_failed) == (2, 1, 1) + # The image: no row, no copy (the dir itself was never created). + assert ( + db.scalar( + select(Document).where( + Document.source == source_root.name, Document.path == "diagram.png" + ) + ) + is None + ) + assert not Path(llm.settings.image_dir).expanduser().exists() + # The text doc indexed as usual (the sync stayed green). + md = db.scalar( + select(Document).where( + Document.source == source_root.name, Document.path == "notes.md" + ) + ) + assert md is not None and md.is_image is False + # The importer's warning names the document (the PLAN §9 signal). + warnings = [ + r + for r in caplog.records + if r.name == "app.importer" and "image description failed" in r.getMessage() + ] + assert len(warnings) == 1 and warnings[0].levelno == logging.WARNING + assert f"source={source_root.name} path=diagram.png" in warnings[0].getMessage() + finally: + _cleanup_source(db, source_root.name) + + +def test_import_sources_images_off_ignores_and_prune_guard_protects( + db, tmp_path, monkeypatch: pytest.MonkeyPatch +) -> None: + """``images=False`` (the default) end-to-end: the walk ignores the + image file entirely (not counted, no row, no copy), and a + ``prune=True`` run MUST NOT delete a pre-existing image doc — the + LOCKED prune guard (invisible to the walk ≠ deleted).""" + source_root = tmp_path / "imgsrc_off" + source_root.mkdir() + (source_root / "diagram.png").write_bytes(PNG_1X1) + _patch_description(monkeypatch, "A network diagram.") + try: + llm_on = _image_llm(tmp_path) + s_on = asyncio.run(import_sources([source_root], llm_on, session=db)) + assert s_on.added == 1 + doc = db.scalar( + select(Document).where( + Document.source == source_root.name, Document.path == "diagram.png" + ) + ) + assert doc is not None + copy = Path(doc.image_path) + assert copy.exists() + + # Toggle OFF (a fresh fake on the code defaults): the walk is + # blind to the file, and prune protects the pre-existing image + # doc + its copy. + llm_off = FakeEmbedder() # Settings(_env_file=None) → images False + s_off = asyncio.run( + import_sources([source_root], llm_off, session=db, prune=True) + ) + assert (s_off.files, s_off.added, s_off.pruned) == (0, 0, 0) + assert db.scalar(select(Document).where(Document.id == doc.id)) is not None + assert copy.exists(), "the copy survives with the doc" + finally: + _cleanup_source(db, source_root.name) + + +def test_import_sources_toggle_on_prunes_deleted_image_with_copy( + db, tmp_path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Toggle ON, the image file deleted: the normal prune runs — the + doc row AND its ``image_dir`` copy are removed (the copy's + lifecycle is tied to the row).""" + source_root = tmp_path / "imgsrc_prune" + source_root.mkdir() + (source_root / "diagram.png").write_bytes(PNG_1X1) + llm_on = _image_llm(tmp_path) + _patch_description(monkeypatch, "A network diagram.") + try: + asyncio.run(import_sources([source_root], llm_on, session=db)) + doc = db.scalar( + select(Document).where( + Document.source == source_root.name, Document.path == "diagram.png" + ) + ) + assert doc is not None + copy = Path(doc.image_path) + assert copy.exists() + + (source_root / "diagram.png").unlink() + s = asyncio.run( + import_sources([source_root], llm_on, session=db, prune=True) + ) + assert (s.pruned, s.added, s.unchanged) == (1, 0, 0) + assert ( + db.scalar(select(Document).where(Document.source == source_root.name)) + is None + ) + assert not copy.exists(), "the image_dir copy is deleted with the doc" + finally: + _cleanup_source(db, source_root.name) + + +# --------------------------------------------------------------------------- +# Phase 122 (task 04) — the image BYTES route + the content/tree wire +# affordance (the serve side of the image-document contract). +# --------------------------------------------------------------------------- + + +def _seed_image_doc( + db, + tmp_path: Path, + *, + source: str = "ImgSrc", + path: str = "pic.png", + data: bytes = PNG_1X1, + content: str = "A red square on a white background.", + image_path: str | None = "auto", + doc_id: str | None = None, +) -> Document: + """One ``is_image`` document row with its persistent copy (the + importer's ``image_dir`` layout) under *tmp_path*; ``image_path`` + ``"auto"`` writes the copy, ``None`` leaves the row without a copy + (the lost-copy corner).""" + doc = Document( + id=uuid.UUID(doc_id) if doc_id else uuid.uuid4(), + source=source, + path=path, + full_path=f"/tmp/{path}", + title=path.rsplit(".", 1)[0], + content=content, + content_hash=hashlib.sha256(data).hexdigest(), + indexed_at=datetime.now(UTC), + created_at=datetime(2026, 1, 1, tzinfo=UTC), + summary=content, + is_image=True, + ) + if image_path == "auto": + copy = tmp_path / f"{doc.id}{Path(path).suffix}" + copy.write_bytes(data) + doc.image_path = str(copy) + elif image_path is not None: + doc.image_path = image_path + db.add(doc) + db.flush() + db.add_all( + [ + Chunk( + document_id=doc.id, position=0, content=content, embedding=[0.01] * 768 + ), + Chunk( + document_id=doc.id, + position=-1, + content=content, + embedding=[0.01] * 768, + is_summary=True, + ), + ] + ) + db.commit() + return doc + + +def _cleanup_kb(db) -> None: + """Truncate the KB tables. The settle commit matters: SQLAlchemy + does NOT autoflush pending ORM objects before a raw ``text()`` + statement — a pending chunks INSERT flushed *after* the TRUNCATE + would FK-violate (the document row is already gone), so any + pending state is committed (then truncated) first.""" + db.commit() + db.execute(text("TRUNCATE chunks, documents")) + db.commit() + + +@pytest.mark.parametrize( + ("ext", "mime"), + [ + (".png", "image/png"), + (".jpg", "image/jpeg"), + (".jpeg", "image/jpeg"), + (".webp", "image/webp"), + (".gif", "image/gif"), + (".bmp", "image/bmp"), + ], +) +def test_image_route_serves_exact_bytes_with_content_type( + admin_client: TestClient, db, tmp_path: Path, ext: str, mime: str +) -> None: + """The serve contract: the route streams the EXACT stored bytes + (a per-extension sentinel — a mix-up between the six formats is + caught) with the extension's ``Content-Type`` (the + ``IMAGE_MIMES`` map — one map, one truth) and + ``Cache-Control: private, max-age=3600`` (content-hashed bytes — + long enough, bustable by re-upload).""" + _cleanup_kb(db) + try: + data = PNG_1X1 + ext.encode("ascii") # per-extension sentinel bytes + doc = _seed_image_doc(db, tmp_path, path=f"pic{ext}", data=data) + r = admin_client.get(f"/api/documents/{doc.id}/image") + assert r.status_code == 200 + assert r.headers["content-type"] == mime # exact type per extension + assert r.headers["cache-control"] == "private, max-age=3600" + assert r.content == data # the exact uploaded bytes, nothing else + finally: + _cleanup_kb(db) + + +def test_image_route_404_matrix(admin_client: TestClient, db, tmp_path: Path) -> None: + """Every non-servable case is 404 ``document not found`` (the + router's unknown-document shape — the same detail string the + content endpoint uses): a missing id, a MALFORMED id (an unparseable + string maps here, not to a 422 — a guessed id is an unknown + document), a text doc, an image doc whose ``image_path`` is NULL, + and a row whose copy was lost on disk (defensive — the row exists, + the bytes don't).""" + _cleanup_kb(db) + try: + _seed_doc(db, "TextSrc", "note.md", "Note", 1, datetime.now(UTC)) + db.commit() # the module's _seed_doc leaves the row uncommitted + text_doc_id = db.scalar( + select(Document.id).where( + Document.source == "TextSrc", Document.path == "note.md" + ) + ) + no_copy = _seed_image_doc(db, tmp_path, path="nopy.png", image_path=None) + lost = _seed_image_doc(db, tmp_path, path="lost.png") + assert lost.image_path is not None # the "auto" copy was written + Path(lost.image_path).unlink() # the copy is lost (the row remains) + + for doc_id in ( + str(uuid.uuid4()), # missing id + "not-a-uuid", # malformed id → 404, not 422 + str(text_doc_id), # text doc + str(no_copy.id), # image doc, image_path NULL + str(lost.id), # image doc, copy lost + ): + r = admin_client.get(f"/api/documents/{doc_id}/image") + assert r.status_code == 404, doc_id + assert r.json() == {"detail": "document not found"}, doc_id + finally: + _cleanup_kb(db) + + +def test_image_route_requires_user_like_the_content_endpoint( + admin_client: TestClient, db, tmp_path: Path +) -> None: + """Phase 79 posture (the task's "PUBLIC, like the document content + endpoint" — the content endpoint has been user-gated since phase + 79; the ONLY anonymous surface is the shared chats, PLAN A10): an + anonymous caller gets 401 ``authentication required`` before any + row is read (a FRESH client — the module's fixture ``client`` + stays unsigned here), a signed-in caller gets the bytes.""" + _cleanup_kb(db) + try: + doc = _seed_image_doc(db, tmp_path) + anonymous = TestClient(fastapi_app) + r = anonymous.get(f"/api/documents/{doc.id}/image") + assert r.status_code == 401 + assert r.json() == {"detail": "authentication required"} + # The signed-in client (admin) passes. + assert admin_client.get(f"/api/documents/{doc.id}/image").status_code == 200 + finally: + _cleanup_kb(db) + + +def test_content_endpoint_exposes_the_image_affordance( + admin_client: TestClient, db, tmp_path: Path +) -> None: + """The content endpoint (the viewer's data source): ``is_image`` is + ALWAYS present (text doc: false — the one new key; the wire shape + gains nothing else), and ``image_url`` — the bytes route's path — + is present for an image doc and ABSENT for a text doc (never null, + the ``DocContent`` omission rule).""" + _cleanup_kb(db) + try: + _seed_doc(db, "TextSrc", "note.md", "Note", 1, datetime.now(UTC)) + db.commit() # the app's endpoint session reads committed data only + r = admin_client.get( + "/api/documents/content", params={"source": "TextSrc", "path": "note.md"} + ) + assert r.status_code == 200 + body = r.json() + assert body["is_image"] is False + assert "image_url" not in body # absent — never null (text doc) + + doc = _seed_image_doc(db, tmp_path, source="ImgSrc", path="pic.png") + r = admin_client.get( + "/api/documents/content", params={"source": "ImgSrc", "path": "pic.png"} + ) + assert r.status_code == 200 + body = r.json() + assert body["is_image"] is True + assert body["image_url"] == f"/api/documents/{doc.id}/image" + assert body["content"] == body["summary"] # the description (task 03) + finally: + _cleanup_kb(db) + + +def _tree_file_nodes_all(sources) -> list[dict]: + """Every file node of a tree response, walked recursively (all + sources — env-registered 0-document sources may join the response + when the ``git_sources`` table is truncated, and they carry no + file nodes; the assertions below hold over whatever files exist). + """ + files: list[dict] = [] + + def _walk(node: dict) -> None: + for child in node.get("children", ()): + if child["kind"] == "file": + files.append(child) + else: + _walk(child) + + for source in sources: + _walk(source) + return files + + +def test_tree_image_file_node_affordance_and_text_node_byte_identical( + admin_client: TestClient, db, tmp_path: Path +) -> None: + """The tree (the RAG view's single fetch): an image doc's file node + carries the thumbnail affordance (``is_image`` true, + ``image_url`` = the bytes route's path, ``summary`` verbatim — the + RAG view's thumbnail ``alt``); EVERY text file node keeps the + pre-phase wire shape byte-identically (the six keys — no + ``is_image``/``image_url``/``summary`` — the phase's + byte-identical criterion: the fields are row-driven, so a KB with + no image rows serializes exactly as pre-phase).""" + _truncate_tree_tables(db) + try: + base = datetime.now(UTC) + _seed_doc(db, "MixedSrc", "a.md", "A", 1, base) + img = _seed_image_doc(db, tmp_path, source="MixedSrc", path="pic.png") + + r = admin_client.get("/api/docs/tree") # both seeds committed (_seed_image_doc) + assert r.status_code == 200 + files = {f["path"]: f for f in _tree_file_nodes_all(r.json()["sources"])} + + # Text node: byte-identical pre-phase wire shape (no image keys). + assert set(files["a.md"]) == { + "kind", "path", "title", "chunks", "created_at", "indexed_at" + } + + # Image node: the affordance rides the node. + pic = files["pic.png"] + assert pic["is_image"] is True + assert pic["image_url"] == f"/api/documents/{img.id}/image" + assert pic["summary"] == "A red square on a white background." + finally: + _truncate_tree_tables(db) + + +def test_tree_with_no_image_rows_is_byte_identical(admin_client: TestClient, db) -> None: + """The pre-phase KB (no ``is_image`` rows): the image-docs map is + empty and EVERY file node serializes in the pre-phase shape — the + row-driven fields introduce no wire change at all (the phase's + byte-identical criterion, the toggle irrelevant).""" + _truncate_tree_tables(db) + try: + base = datetime.now(UTC) + _seed_doc(db, "PlainSrc", "x.md", "X", 2, base) + db.commit() # the app's endpoint session reads committed data only + r = admin_client.get("/api/docs/tree") + assert r.status_code == 200 + files = _tree_file_nodes_all(r.json()["sources"]) + assert len(files) == 1 # the seeded doc (env sources carry no files) + assert set(files[0]) == { + "kind", "path", "title", "chunks", "created_at", "indexed_at" + } + finally: + _truncate_tree_tables(db) diff --git a/tests/integration/test_document_content.py b/tests/integration/test_document_content.py index 4c79eca..14a2a85 100644 --- a/tests/integration/test_document_content.py +++ b/tests/integration/test_document_content.py @@ -422,11 +422,16 @@ def test_content_200_all_fields(client, db) -> None: assert r.status_code == 200 body = r.json() # Wire-additive (phase 106, task 05): ``created_at`` joins the - # content shape (after ``summary``, before ``content``). + # content shape (after ``summary``, before ``content``) — and + # (phase 122, task 04) ``is_image`` joins it ALWAYS present + # (text docs: false); ``image_url`` is ABSENT for a text doc + # (never null — the ``DocContent`` omission rule). assert set(body) == { "source", "path", "title", "format", "summary", "created_at", - "content", "indexed_at", "chunks", + "content", "indexed_at", "chunks", "is_image", } + assert body["is_image"] is False + assert "image_url" not in body # absent — never null (text doc) datetime.fromisoformat(body["created_at"]) # raises if not ISO-8601 assert body["source"] == "Homelab" assert body["path"] == "kubernetes.md" diff --git a/tests/integration/test_migration_0022.py b/tests/integration/test_migration_0022.py new file mode 100644 index 0000000..172ccc2 --- /dev/null +++ b/tests/integration/test_migration_0022.py @@ -0,0 +1,344 @@ +"""Integration: migration 0022 (documents.is_image + image_path) schema +contract (phase 122, task 02). + +Drives the **real Alembic engine** against the live dev database +(``podman compose up -d db``), mirroring the house pattern of +``test_migration_0021.py`` (information_schema assertions on the state +the migration must leave). The tests target the 0021 → 0022 step +explicitly so later migrations cannot break the pins: + +* upgrade 0021 → 0022 → ``is_image`` exists with the full contract — + BOOLEAN, NOT NULL, server default ``false`` — and ``image_path`` — + TEXT, NULLABLE, no server default — while the 0021 ``documents`` + schema (``content``/``content_hash`` NOT NULL, ``summary`` NULLABLE, + ``created_at`` + ``created_at_manual``, the (source, path) unique + constraint — asserted column-based, since the suite's table + self-heal renames copied constraints) survives; +* a ``documents`` row inserted while the DB is at 0021 backfills + ``is_image`` to ``false`` and ``image_path`` to NULL (every + pre-phase-122 row is a text doc — the LOCKED A3 default); +* the ORM contract agrees: a freshly inserted ``Document`` without the + image fields reads ``is_image is False`` / ``image_path is None``, + and one with them round-trips through a fresh session; +* downgrade to 0021 → both columns are GONE (A13) while the row + survives; upgrade back to 0022 → the columns are back (round-trip). + +The ``alembic`` fixture guarantees the DB ends at head even if a test +fails or the process is interrupted. +""" +from __future__ import annotations + +import uuid +from collections.abc import Iterator +from typing import Any + +import pytest +from alembic.config import Config +from sqlalchemy import text +from sqlalchemy.orm import Session + +from alembic import command +from app.db import SessionLocal, db_available +from app.models import Document + +SOURCE = "mig0022" +PATH_TEXT = "notes/readme.md" +PATH_IMAGE = "notes/diagram.png" + + +@pytest.fixture() +def alembic(db: Session) -> Iterator[Config]: + """Real Alembic config bound to the dev DB (URL from app settings). + + Starts at head (repairs an interrupted earlier run); teardown + upgrades to head no matter what happened, so the dev DB is never + left below head. + """ + if not db_available(): + pytest.skip("Postgres not reachable — run `podman compose up -d db` first") + cfg = Config() # no alembic.ini file — env.py gets the URL from app config + cfg.set_main_option("script_location", "alembic") + command.upgrade(cfg, "head") + try: + yield cfg + finally: + # Release the test session's open transaction BEFORE the repair + # DDL: an idle-in-transaction SELECT holds an ACCESS SHARE lock + # on ``documents``, which would deadlock the repair's + # ``ALTER TABLE`` (0022) forever. + db.rollback() + command.upgrade(cfg, "head") + + +def _version(db: Session) -> str | None: + return db.execute(text("SELECT version_num FROM alembic_version")).scalar() + + +def _column(db: Session, column: str) -> tuple[Any, ...] | None: + """(data_type, is_nullable, column_default) for one documents + column.""" + row = db.execute( + text( + "SELECT data_type, is_nullable, column_default" + " FROM information_schema.columns" + " WHERE table_name = 'documents' AND column_name = :c" + ), + {"c": column}, + ).fetchone() + return tuple(row) if row is not None else None + + +def _insert_sql(db: Session, path: str) -> uuid.UUID: + """Insert one documents row with the PRE-0022 column set (the 0021 + shape — the image columns, when present, are omitted so their + backfill is what the row reads).""" + row_id = uuid.uuid4() + db.execute( + text( + "INSERT INTO documents (id, source, path, full_path, title," + " content, content_hash)" + " VALUES (:id, :s, :p, :f, :t, :c, :h)" + ), + { + "id": row_id, + "s": SOURCE, + "p": path, + "f": f"/tmp/{SOURCE}/{path}", + "t": path.rsplit("/", 1)[-1], + "c": "content", + "h": "0" * 64, + }, + ) + db.commit() + return row_id + + +def _delete(db: Session, row_id: uuid.UUID) -> None: + db.execute(text("DELETE FROM documents WHERE id = :id"), {"id": row_id}) + db.commit() + + +def _unique_column_sets(db: Session) -> set[tuple[str, ...]]: + """The column tuples of every UNIQUE constraint on ``documents``. + + Column-based (not name-based): the integration suite's table + self-heal (``tests/integration/conftest.py``) rewrites bloated + tables via ``CREATE TABLE (LIKE …)``, which renames copied + constraints (PG auto-names them) — the (source, path) uniqueness + contract is what must hold, not the original name. + """ + rows = db.execute( + text( + "SELECT (SELECT string_agg(a.attname, ',' ORDER BY k.ord)" + " FROM unnest(c.conkey) WITH ORDINALITY k(attnum, ord)" + " JOIN pg_attribute a" + " ON a.attrelid = c.conrelid AND a.attnum = k.attnum)" + " FROM pg_constraint c" + " WHERE c.contype = 'u' AND c.conrelid = 'documents'::regclass" + ) + ).fetchall() + return {tuple(r[0].split(",")) for r in rows} + + +def test_upgrade_to_0022_adds_image_columns(db: Session, alembic: Config) -> None: + """Upgrade 0021 → 0022: ``is_image`` exists with the full contract + (BOOLEAN, NOT NULL, server default ``false`` — every pre-phase-122 + row is a text doc) and ``image_path`` (TEXT, NULLABLE, no server + default — NULL for text docs), both ABSENT at 0021; a pre-0022 row + backfills ``is_image`` to ``false`` + ``image_path`` to NULL; and + the 0021 table contract survives the additive upgrade.""" + command.downgrade(alembic, "0021") # start from the pre-0022 state + assert _version(db) == "0021" + assert _column(db, "is_image") is None, "is_image must be absent at 0021" + assert _column(db, "image_path") is None, "image_path must be absent at 0021" + + pre_id = _insert_sql(db, PATH_TEXT) # the 0021 column set + try: + command.upgrade(alembic, "0022") + assert _version(db) == "0022", "alembic_version must be at 0022" + + is_image = _column(db, "is_image") + assert is_image is not None, "documents.is_image is missing" + assert is_image[0] == "boolean", "is_image must be BOOLEAN" + assert is_image[1] == "NO", "is_image must be NOT NULL" + assert is_image[2] == "false", ( + "is_image must carry the `false` server default — every" + " pre-phase-122 row is a text doc" + ) + + image_path = _column(db, "image_path") + assert image_path is not None, "documents.image_path is missing" + assert image_path[0] == "text", "image_path must be TEXT" + assert image_path[1] == "YES", "image_path must be NULLABLE" + assert image_path[2] is None, ( + "image_path must carry NO server default — NULL is the" + " text-doc value" + ) + + # The pre-0022 row backfilled to (false, NULL) — a text doc. + row = db.execute( + text("SELECT is_image, image_path FROM documents WHERE id = :id"), + {"id": pre_id}, + ).fetchone() + assert row is not None, "the pre-0022 row must survive the upgrade" + assert row[0] is False, "the backfilled is_image must be false" + assert row[1] is None, "the backfilled image_path must be NULL" + + # A row written without the image columns reads the same + # (the Python-side defaults are False/None — same values). + new_id = _insert_sql(db, PATH_IMAGE) + try: + backfilled = db.execute( + text("SELECT is_image, image_path FROM documents WHERE id = :id"), + {"id": new_id}, + ).fetchone() + assert backfilled == (False, None), ( + "an omitted image state must read (false, NULL)" + ) + finally: + _delete(db, new_id) + + # The 0021 schema survives the additive upgrade. + content = _column(db, "content") + assert content is not None and content[0] == "text" and content[1] == "NO", ( + "documents.content (0001) must keep its 0021 contract" + ) + hash_col = _column(db, "content_hash") + assert ( + hash_col is not None + and hash_col[0] == "character varying" + and hash_col[1] == "NO" + ), "documents.content_hash (0001) must survive the upgrade" + summary = _column(db, "summary") + assert summary is not None and summary[0] == "text" and summary[1] == "YES", ( + "documents.summary (phase 30) must survive the upgrade" + ) + created = _column(db, "created_at") + assert created is not None and created[0] == "timestamp with time zone" + assert created[1] == "NO" and "now()" in str(created[2]), ( + "documents.created_at (0020) must keep its `now()` server default" + ) + manual = _column(db, "created_at_manual") + assert manual is not None and manual[0] == "boolean" and manual[1] == "NO" + assert manual[2] == "false", ( + "documents.created_at_manual (0020) must keep its `false` default" + ) + assert ("source", "path") in _unique_column_sets(db), ( + "the (source, path) unique constraint must survive the upgrade" + ) + finally: + _delete(db, pre_id) + + +def test_orm_image_fields_round_trip(db: Session, alembic: Config) -> None: + """The ORM contract agrees with the column contract: a freshly + inserted ``Document`` WITHOUT the image fields reads ``is_image is + False`` / ``image_path is None`` (the text-doc default state), and + one WITH them round-trips the pair through a FRESH session.""" + command.upgrade(alembic, "head") + text_doc = Document( + source=SOURCE, + path=PATH_TEXT, + full_path=f"/tmp/{SOURCE}/{PATH_TEXT}", + title="readme", + content="# readme\n", + content_hash="1" * 64, + ) + image_doc = Document( + source=SOURCE, + path=PATH_IMAGE, + full_path=f"/tmp/{SOURCE}/{PATH_IMAGE}", + title="diagram", + content="A description of the diagram.", + content_hash="2" * 64, + is_image=True, + image_path="/srv/bor-images/diagram.png", + ) + db.add(text_doc) + db.add(image_doc) + db.commit() + try: + with SessionLocal() as fresh: + reloaded_text = fresh.get(Document, text_doc.id) + assert reloaded_text is not None, "the text row must be readable" + assert reloaded_text.is_image is False, ( + "an omitted is_image must read the False default" + ) + assert reloaded_text.image_path is None, ( + "an omitted image_path must read NULL" + ) + reloaded_image = fresh.get(Document, image_doc.id) + assert reloaded_image is not None, "the image row must be readable" + assert reloaded_image.is_image is True + assert reloaded_image.image_path == "/srv/bor-images/diagram.png" + finally: + _delete(db, text_doc.id) + _delete(db, image_doc.id) + + +def test_downgrade_to_0021_drops_the_columns(db: Session, alembic: Config) -> None: + """Downgrade 0022 → 0021: both image columns are gone (A13 — fully + reversible) while the row + its 0021 columns survive, and the rest + of the 0021 table contract (``content``, ``content_hash``, + ``created_at``) is intact.""" + command.upgrade(alembic, "head") + row = Document( + source=SOURCE, + path=PATH_IMAGE, + full_path=f"/tmp/{SOURCE}/{PATH_IMAGE}", + title="diagram", + content="A description of the diagram.", + content_hash="3" * 64, + is_image=True, + image_path="/srv/bor-images/diagram.png", + ) + db.add(row) + db.commit() + try: + command.downgrade(alembic, "0021") + assert _version(db) == "0021" + assert _column(db, "is_image") is None, "is_image must be dropped" + assert _column(db, "image_path") is None, "image_path must be dropped" + + surviving = db.execute( + text( + "SELECT source, path, title, content, content_hash, created_at" + " FROM documents WHERE id = :id" + ), + {"id": row.id}, + ).fetchone() + assert surviving is not None, "the row must survive the column drops" + assert surviving[0] == SOURCE and surviving[1] == PATH_IMAGE + assert surviving[3] == "A description of the diagram." + assert surviving[4] == "3" * 64 + assert surviving[5] is not None, "created_at must survive the drops" + + assert ("source", "path") in _unique_column_sets(db), ( + "the (source, path) unique constraint must survive the downgrade" + ) + finally: + _delete(db, row.id) + # Repair: the fixture teardown re-upgrades to head. + + +def test_upgrade_round_trip_restores_the_columns(db: Session, alembic: Config) -> None: + """Downgrade to 0021, then upgrade back to 0022: both columns are + back with the full contract (``is_image`` BOOLEAN NOT NULL default + ``false``; ``image_path`` TEXT NULLABLE no default).""" + command.downgrade(alembic, "0021") + command.upgrade(alembic, "0022") + assert _version(db) == "0022", "round-trip upgrade must land at 0022" + + is_image = _column(db, "is_image") + assert is_image is not None, "documents.is_image must be back" + assert is_image[0] == "boolean", "is_image must be BOOLEAN after the round-trip" + assert is_image[1] == "NO", "is_image must be NOT NULL after the round-trip" + assert is_image[2] == "false", ( + "is_image must still carry the `false` server default" + ) + + image_path = _column(db, "image_path") + assert image_path is not None, "documents.image_path must be back" + assert image_path[0] == "text" + assert image_path[1] == "YES" + assert image_path[2] is None, "image_path must still carry NO server default" diff --git a/tests/integration/test_ui_settings_api.py b/tests/integration/test_ui_settings_api.py index e31c1d2..dec0d6e 100644 --- a/tests/integration/test_ui_settings_api.py +++ b/tests/integration/test_ui_settings_api.py @@ -216,10 +216,11 @@ def test_admin_semantic_fields_round_trip(client: TestClient, db: Session) -> No def _config_keys() -> set[str]: - """The /api/config key set after task 03: the five phase-39/59/62 - keys — the retired CSS-file theming's ``theme`` key is gone.""" + """The /api/config key set after task 03 (phase 91): the retired + CSS-file theming's ``theme`` key is gone; phase 122 (task 01) added + the ``images`` flag — the six keys below are the contract.""" return {"app_name", "version", "docs_repo_configured", - "input_placeholder", "footer_text"} + "images", "input_placeholder", "footer_text"} def test_api_config_env_only_deployment_returns_env_strings(client: TestClient) -> None: diff --git a/tests/unit/test_document_viewer.py b/tests/unit/test_document_viewer.py index 67bf1c5..d9eabf4 100644 --- a/tests/unit/test_document_viewer.py +++ b/tests/unit/test_document_viewer.py @@ -179,6 +179,7 @@ def test_content_known_pair_maps_to_doc_content() -> None: title="Deep Mark", content="# Deep Mark\n\nbody", content_hash="f" * 64, + is_image=False, # phase 122: the stub session never applies defaults ) doc.indexed_at = datetime(2026, 8, 22, 1, 2, 3, tzinfo=UTC) doc.created_at = datetime(2026, 8, 20, 9, 0, 0, tzinfo=UTC) # phase 106 @@ -190,11 +191,15 @@ def test_content_known_pair_maps_to_doc_content() -> None: assert r.status_code == 200 body = r.json() # Wire-additive (phase 106, task 05): the pre-date keys are all - # still there, joined by ``created_at``. + # still there, joined by ``created_at`` — and (phase 122, task 04) + # by ``is_image`` (ALWAYS present; text docs: false) while + # ``image_url`` is ABSENT (never null — the omission rule). assert set(body) == { "source", "path", "title", "format", "summary", "created_at", - "content", "indexed_at", "chunks", + "content", "indexed_at", "chunks", "is_image", } + assert body["is_image"] is False + assert "image_url" not in body # absent — never null (text doc) assert body["source"] == "Homelab" assert body["path"] == "notes/deep mark.md" assert body["title"] == "Deep Mark" diff --git a/tests/unit/test_image_documents.py b/tests/unit/test_image_documents.py new file mode 100644 index 0000000..3a94fa7 --- /dev/null +++ b/tests/unit/test_image_documents.py @@ -0,0 +1,1396 @@ +"""Unit: phase 122 — image documents as first-class, retrievable docs. + +Task 01 (the toggle): the single env knob for image support +(``BOR_IMAGES``) plus its two companions (``BOR_IMAGE_EXTENSIONS`` / +``BOR_IMAGE_DIR``): defaults (LOCKED A3 — off by default), env +overrides, and the dotted frozenset parse the walk filter will consume +(the ``import_extension_set`` precedent). + +Task 02 (the ingest): the walk admits image extensions ONLY when the +image set is passed (the ``images`` toggle on); the binary index path +(bytes digest, persistent copy into ``image_dir``, ``is_image``/ +``image_path``, ``content`` = the description, ``read_text`` never +called); the stale-copy delete on a content change; the LOCKED prune +guard (images-off runs never prune ``is_image`` docs; images-on runs +prune a deleted image file + its copy); and the fail-soft skip +(failed description → no row, no orphan copy, ``images_failed`` + +warning + the counter in the PLAN §9 line). + +Task 03 (the description): ``summarizer.describe_image`` — ONE +CHAT-model (vision) call (LOCKED A3 — the ``lite`` summary model is +NOT assumed vision-capable) with the OpenAI-compatible multimodal user +message (the fixed prompt's text part + the image's data-URL part), +the reply stripped + capped at ``summary_max_chars``, ``None`` on any +client error / empty reply. The seam ``importer._describe_or_skip`` is +filled (the tests here exercise BOTH the patched seam — the +importer-side mechanics — and the REAL seam through a mock vision +client): the description becomes ``content`` AND ``summary`` (the +``is_summary`` chunk mirrors it — the ONLY embedded text of the doc), +a failed description skips the doc (no row, no copy — and a CHANGED +image's old row stays as-is), and the phase-118 backfill for an +unchanged image doc re-stores the stored description verbatim. + +Task 04 (serve + display): the content endpoint's wire affordance +(``is_image`` always present, ``image_url`` absent for text docs — +never null) and the tree file node's omission rule (a text node +serializes byte-identically to pre-phase), plus the house-style +frontend source contracts: the viewer renders the ```` from +``doc.image_url`` with ``alt = summary`` + the onerror note, and the +Sources row's 48px lazy thumbnail falls back to the document glyph on +a failed fetch (the glyph swap keeps the module's single-innerHTML +pin). The ext→mime map pin (task 03's ``IMAGE_MIMES`` — one map, one +truth with the serve route's ``Content-Type``). + +Task 05 (the RAG display): the shared frame builder +(``retriever.source_ref_with_image`` — the chat API's cited AND +related tiers both run through it) carries the OPTIONAL ``image_url`` +(the bytes route) on an ``is_image`` doc's ref and OMITS the key for +a text doc (never null — the text-doc frame stays byte-identical, the +nested-``done``-frame pin included); the agent ``read`` result for an +image doc prefacing the description with the pinned +``IMAGE_DOC_MARKER`` line (header + date line byte-identical, the +cap-truncation case marker-before-cut, a text doc's result +byte-identical); and the house-style frontend source contracts — the +chat sources block's inline image figure (both the citation chips and +the related row, gated on ``s.image_url``) reads ``image_url``, sets +``alt`` + the visible caption to the document's summary (fetched from +the content endpoint the chip's modal already uses — the frame +carries no summary), and collapses to the plain chip on an image +load error (never a broken-image icon). The SSE-level byte pin sits in +``tests/integration/test_chat_api.py``; the E2E rendering scenario is +task 06's isolated story suite. +""" +from __future__ import annotations + +import asyncio +import base64 +import logging +import re +from pathlib import Path +from typing import Any + +import pytest +from pydantic import ValidationError +from sqlalchemy import select + +import app.rag.importer as importer +from app.config import Settings +from app.models import Chunk, Document +from app.rag.importer import import_sources, iter_importable_files +from app.rag.llm import EmbeddingError, LLMError +from app.rag.summarizer import ( + DESCRIBE_PROMPT, + IMAGE_DESCRIPTION_MODE, + IMAGE_FALLBACK_MIME, + IMAGE_MIMES, + describe_image, +) +from tests.fakes import FakeEmbedder + + +def _settings(**kwargs: Any) -> Settings: + """Build Settings without reading a .env file (deterministic tests).""" + kwargs.setdefault("_env_file", None) + return Settings(**kwargs) # pyright: ignore[reportCallIssue] (kwarg exists at runtime) + + +def test_image_defaults_match_locked_a3() -> None: + """LOCKED A3: image support is OFF by default (enable only when the + chat model supports vision), the built-in extension set is the six + common formats, and the image bytes' persistent home is the + ``~/bor-sources/images`` raw string (``expanduser`` is the + importer's job — the ``sources_dir``/``upload_dir`` convention).""" + s = _settings() + assert s.images is False + # Dotted, lowercased form — the shape the walk's matcher consumes + # (the ``import_extension_set`` precedent). + assert s.image_extension_set == {".png", ".jpg", ".jpeg", ".webp", ".gif", ".bmp"} + assert s.image_dir == "~/bor-sources/images" + # The raw string is stored untouched (no expanduser here). + assert s.image_extensions == "png,jpg,jpeg,webp,gif,bmp" + + +def test_image_set_is_separate_from_the_import_set() -> None: + """Images are never user-added via ``BOR_IMPORT_EXTENSIONS`` — the + ``images`` toggle is the single knob (LOCKED A3): an override of + the import set leaves the image set untouched, and vice versa.""" + s = _settings(import_extensions="md") + assert s.import_extension_set == {".md"} + assert s.image_extension_set == {".png", ".jpg", ".jpeg", ".webp", ".gif", ".bmp"} + s2 = _settings(image_extensions="md") + assert s2.image_extension_set == {".md"} + assert ".md" in s2.import_extension_set # the default import set is intact + + +def test_images_env_toggle(monkeypatch: pytest.MonkeyPatch) -> None: + """``BOR_IMAGES`` follows the house bool convention: ``1``/``true`` + on, ``0``/``false`` off (case-insensitive), unset → off.""" + monkeypatch.delenv("BOR_IMAGES", raising=False) + assert _settings().images is False + for on in ("1", "true", "TRUE", "yes"): + monkeypatch.setenv("BOR_IMAGES", on) + assert _settings().images is True + for off in ("0", "false", "FALSE", "no"): + monkeypatch.setenv("BOR_IMAGES", off) + assert _settings().images is False + + +def test_image_extensions_env_override(monkeypatch: pytest.MonkeyPatch) -> None: + """``BOR_IMAGE_EXTENSIONS`` overrides the CSV; the frozenset parse + is case-insensitive, trims spaces, and normalizes a leading dot.""" + monkeypatch.setenv("BOR_IMAGE_EXTENSIONS", "PNG, Jpeg , .webp") + s = _settings() + assert s.image_extensions == "PNG, Jpeg , .webp" # stored raw + assert s.image_extension_set == {".png", ".jpeg", ".webp"} + monkeypatch.setenv("BOR_IMAGE_EXTENSIONS", "tiff") + assert _settings().image_extension_set == {".tiff"} + + +def test_image_dir_env_override(monkeypatch: pytest.MonkeyPatch) -> None: + """``BOR_IMAGE_DIR`` overrides the raw string (expanduser stays the + importer's job — no Path resolution in Settings).""" + monkeypatch.setenv("BOR_IMAGE_DIR", "/srv/bor-images") + assert _settings().image_dir == "/srv/bor-images" + + +def test_image_extensions_rejects_empty(monkeypatch: pytest.MonkeyPatch) -> None: + """A blank list would index zero images silently — fail loud at + startup, naming the value (the ``import_extensions`` precedent).""" + monkeypatch.setenv("BOR_IMAGE_EXTENSIONS", " , ") + with pytest.raises(ValidationError, match="image_extensions"): + _settings() + + +def test_image_extensions_rejects_malformed_tokens( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A malformed token (e.g. a ``jpeb`` typo with punctuation) fails + startup loudly, naming the bad token(s) — the same shape guard as + ``import_extensions``.""" + monkeypatch.setenv("BOR_IMAGE_EXTENSIONS", "png,jpe b") + with pytest.raises(ValidationError, match="jpe b"): + _settings() + + +# --------------------------------------------------------------------------- +# Phase 122, task 02 — the image ingest: walk filter, binary branch, copy +# lifecycle, prune guard, fail-soft skip. +# --------------------------------------------------------------------------- + +#: A real 1×1 transparent PNG — the importer is content-agnostic (it +#: never parses the image), but a well-formed fixture keeps the tests +#: honest about what a real upload looks like. +PNG_1X1 = base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAC0lEQVR4nGP4DwQACfsD/fteaysAAAAASUVORK5CYII=" +) + + +class _SummaryEmbedFailsLLM(FakeEmbedder): + """``embed`` succeeds on the content batch (the first) and FAILS on + the second — the phase-30 ``is_summary`` chunk: the image doc lands + indexed (its content chunks are already committed), its + ``documents.summary`` stays NULL (the fail-soft summary path), and + the phase-118 backfill trigger arms. (Task 03: for an image the + summary is the stored description — no ``lite`` chat call exists in + that path anymore, so the summary chunk's embed is the one + remaining failure class.)""" + + def __init__(self) -> None: + super().__init__() + self._embed_batches = 0 + + async def embed(self, texts: list[str]) -> list[list[float]]: + self._embed_batches += 1 + if self._embed_batches == 2: + raise EmbeddingError( + "simulated summary-chunk embed failure (test sentinel)" + ) + return await super().embed(texts) + + +class _VisionMockLLM(FakeEmbedder): + """The phase-122 mock VISION client (task 03): the CHAT model + answers the multimodal describe call (the image bytes' data URL) + with a fixed, retrieval-oriented description; text (``lite``) calls + keep the ``FakeEmbedder`` behaviour. The REAL ``_describe_or_skip`` + → ``describe_image`` chain runs against it (no seam patch); every + chat call's model is recorded (``chat_models``).""" + + DESCRIPTION = ( + "A network diagram of the homelab VLANs: the core switch, the " + "router, and three labeled subnets." + ) + + def __init__(self) -> None: + super().__init__() + self.chat_models: list[str | None] = [] + + async def chat( + self, messages: list[dict[str, Any]], model: str | None = None + ) -> str: + self.chat_calls.append(list(messages)) + self.chat_models.append(model) + user = next((m["content"] for m in messages if m.get("role") == "user"), "") + if isinstance(user, list): + # The phase-122 describe call — the multimodal message. + return self.DESCRIPTION + first = user.split() + return "Summary of " + (first[0] if first else "") + + +class _VisionFailsLLM(FakeEmbedder): + """A chat model that REJECTS the multimodal describe call (the + non-vision model of the LOCKED A3 note — the SDK errors; every + image is skipped + logged, the honest visible failure). Text calls + keep the ``FakeEmbedder`` behaviour.""" + + async def chat( + self, messages: list[dict[str, Any]], model: str | None = None + ) -> str: + self.chat_calls.append(list(messages)) + user = next((m["content"] for m in messages if m.get("role") == "user"), "") + if isinstance(user, list): + raise LLMError("simulated non-vision chat model (test sentinel)") + first = user.split() + return "Summary of " + (first[0] if first else "") + + +def _image_settings(tmp_path: Path, **kwargs: Any) -> Settings: + """Phase-122 image settings (toggle ON by default; the image dir + defaults under *tmp_path* unless overridden).""" + kwargs.setdefault("_env_file", None) + kwargs.setdefault("images", True) + kwargs.setdefault("image_dir", str(tmp_path / "images")) + return Settings(**kwargs) # pyright: ignore[reportCallIssue] + + +def _image_llm[ + ImageLLM: FakeEmbedder +](tmp_path: Path, llm_cls: type[ImageLLM] = FakeEmbedder, **kwargs: Any) -> ImageLLM: + """A fake LLM whose settings carry the phase-122 image knobs. + + *llm_cls* (task 03) may be a mock vision client (``FakeEmbedder`` + subclass carrying extra state such as ``chat_models``) — the PEP 695 + type parameter keeps the helper's return type honest.""" + llm = llm_cls() + llm.settings = _image_settings(tmp_path, **kwargs) + return llm + + +def _patch_description( + monkeypatch: pytest.MonkeyPatch, description: str | None +) -> list[bytes]: + """Pin the task-02 seam (``importer._describe_or_skip``) to + *description* and record the BYTES it is handed (the digest-rule + pin). Task 03 replaces the seam's body — the importer-side + mechanics under test here do not change with it.""" + handed: list[bytes] = [] + + async def _fake( + llm: importer.Embedder, *, data: bytes, source: str, rel: str, full_path: Path + ) -> str | None: + handed.append(data) + return description + + monkeypatch.setattr(importer, "_describe_or_skip", _fake) + return handed + + +def _doc(db, source: str, rel: str) -> Document: + doc = db.scalar( + select(Document).where(Document.source == source, Document.path == rel) + ) + assert doc is not None, f"no documents row for ({source!r}, {rel!r})" + return doc + + +def _cleanup_source(db, source: str) -> None: + for doc in db.scalars(select(Document).where(Document.source == source)).all(): + db.delete(doc) + db.commit() + + +# --- the walk filter ------------------------------------------------------- + + +def test_walk_accepts_images_only_when_the_image_set_is_passed( + tmp_path: Path, +) -> None: + """The walk admits image extensions ONLY when the caller passes the + image set (``import_sources`` does so while the ``images`` toggle is + on): with an empty set (the default — toggle off) a ``.png`` is + invisible, byte-identical to the pre-phase walk.""" + root = tmp_path / "imgroot" + root.mkdir() + (root / "a.md").write_text("# A\n\nbody\n", encoding="utf-8") + (root / "pic.png").write_bytes(PNG_1X1) + import_set = frozenset({".md", ".txt"}) + assert [p.name for p in iter_importable_files(root, import_set)] == ["a.md"] + got = [ + p.name + for p in iter_importable_files( + root, import_set, image_extensions=frozenset({".png", ".jpg"}) + ) + ] + assert got == ["a.md", "pic.png"] # rglob is sorted + # An image-set file never enters the import set (no ``md``-style + # bleed): the two sets stay separate (LOCKED A3). + assert [ + p.name + for p in iter_importable_files( + root, import_set, image_extensions=frozenset({".gif"}) + ) + ] == ["a.md"] + + +def test_images_off_run_is_byte_identical_walk(db, tmp_path: Path) -> None: + """With the toggle OFF, ``import_sources`` walks the pre-phase set — + a ``.png`` is not even counted as a file (not ``unknown``, not an + error, no image row).""" + root = tmp_path / "offimg" + root.mkdir() + (root / "a.md").write_text("# A\n\nbody\n", encoding="utf-8") + (root / "pic.png").write_bytes(PNG_1X1) + llm = _image_llm(tmp_path, images=False) + try: + summary = asyncio.run(import_sources([root], llm, session=db)) + assert (summary.files, summary.added, summary.images_failed) == (1, 1, 0) + assert summary.formats == {"md": 1} # never ``unknown``, never ``png`` + assert ( + db.scalar( + select(Document).where( + Document.source == root.name, Document.path == "pic.png" + ) + ) + is None + ) + finally: + _cleanup_source(db, root.name) + + +# --- the binary index path -------------------------------------------------- + + +def test_image_binary_branch_digests_bytes_copies_and_sets_fields( + db, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The binary branch: the sha256 is over the raw BYTES (the digest + rule is content identity), the copy lands in ``image_dir`` as + ``.png``, ``is_image``/``image_path`` are set, ``content`` + is the description (the ONLY embedded text — the embedding model + never sees pixels), and ``read_text`` is NEVER called for an image.""" + root = tmp_path / "imgsrc" + root.mkdir() + (root / "pic.png").write_bytes(PNG_1X1) + llm = _image_llm(tmp_path) + handed = _patch_description(monkeypatch, "A red square on a white background.") + + def _no_read_text(self, *args: Any, **kwargs: Any) -> None: + raise AssertionError(f"read_text called for an image file: {self}") + + monkeypatch.setattr(Path, "read_text", _no_read_text) + try: + summary = asyncio.run(import_sources([root], llm, session=db)) + assert (summary.files, summary.added, summary.images_failed) == (1, 1, 0) + assert summary.formats == {"png": 1} # counted under its image token + assert handed == [PNG_1X1] # the seam digests the BYTES, not text + + doc = _doc(db, root.name, "pic.png") + assert doc.is_image is True + assert doc.title == "pic" # the non-markdown stem rule + assert doc.content == "A red square on a white background." + # Task 03: the description IS the summary (stored verbatim — + # no ``lite`` call, no pointer line). + assert doc.summary == doc.content + assert doc.image_path is not None + copy = Path(doc.image_path) + assert copy.parent == Path(llm.settings.image_dir).expanduser() + assert copy.name == f"{doc.id}.png" # row and copy agree on the id + assert copy.read_bytes() == PNG_1X1 + # The ONLY embedded text is the description: every content chunk + # carries it, and every chunk (content + the phase-30 summary + # chunk) has a 768-dim vector. + chunks = db.scalars(select(Chunk).where(Chunk.document_id == doc.id)).all() + content_chunks = [c for c in chunks if not c.is_summary] + assert [c.content for c in content_chunks] == [doc.content] + for c in chunks: + assert c.embedding is not None and len(c.embedding) == 768 + finally: + _cleanup_source(db, root.name) + + +def test_failed_description_skips_the_doc_counts_and_logs( + db, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """LOCKED A3 fail-soft: a failed/empty description skips the doc + entirely — no row, NO copy (the copy is only made after a + successful description, so a failure never leaves an orphan — not + even the dir), ``images_failed`` is bumped, a warning is logged, + and the counter sits in the PLAN §9 summary line.""" + root = tmp_path / "failimg" + root.mkdir() + (root / "pic.png").write_bytes(PNG_1X1) + llm = _image_llm(tmp_path) + _patch_description(monkeypatch, None) + try: + with caplog.at_level(logging.INFO, logger="app.importer"): + summary = asyncio.run(import_sources([root], llm, session=db)) + finally: + _cleanup_source(db, root.name) + + assert (summary.files, summary.added, summary.images_failed) == (1, 0, 1) + assert ( + db.scalar(select(Document).where(Document.source == root.name)) is None + ), "a failed description must leave NO row" + assert not Path(llm.settings.image_dir).expanduser().exists(), ( + "no copy — and not even the image dir — may be left behind" + ) + warnings = [ + r for r in caplog.records if "image description failed" in r.getMessage() + ] + assert len(warnings) == 1 and warnings[0].levelno == logging.WARNING + line = next( + r.getMessage() for r in caplog.records if "import: summary files=" in r.getMessage() + ) + assert "images_failed=1" in line + + +def test_unchanged_image_skips_and_backfills_null_summary( + db, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An unchanged image (byte digest) is skipped — the seam is NOT + called again, no content re-embed, no new copy — and an unchanged + image doc whose summary is still NULL (an earlier fail-soft summary + miss — for an image, the one remaining failure class after task 03: + the ``is_summary`` chunk's embed) gets the phase-118 best-effort + summary pass (image flavour: the summary is the stored description, + stored verbatim — no ``lite`` call); the counters stay + ``summary_backfilled``, never added/updated.""" + root = tmp_path / "unchimg" + root.mkdir() + (root / "pic.png").write_bytes(PNG_1X1) + handed = _patch_description(monkeypatch, "A red square.") + try: + # First import: the description succeeds but the summary + # chunk's embed fails → the doc is indexed, its summary stays + # NULL (the fail-soft summary path). + llm1 = _image_llm(tmp_path, _SummaryEmbedFailsLLM) + s1 = asyncio.run(import_sources([root], llm1, session=db)) + assert (s1.added, s1.summaries, s1.summary_errors) == (1, 0, 1) + assert len(handed) == 1 and handed[0] == PNG_1X1 + doc = _doc(db, root.name, "pic.png") + assert doc.content == "A red square." # the seam's description + assert doc.summary is None + assert doc.image_path is not None + copy = Path(doc.image_path) + assert copy.exists() + + # Unchanged re-run with a working LLM. + llm2 = _image_llm(tmp_path) + s2 = asyncio.run(import_sources([root], llm2, session=db)) + assert (s2.unchanged, s2.added, s2.updated) == (1, 0, 0) + assert s2.summary_backfilled == 1 and s2.images_failed == 0 + assert len(handed) == 1, "the seam is NOT called on the unchanged run" + assert len(llm2.calls) == 1, "no content re-embed — the summary chunk only" + + doc2 = _doc(db, root.name, "pic.png") + assert doc2.summary is not None, "the NULL summary is backfilled" + assert doc2.summary == doc2.content, "image summary = the stored description" + schunks = [c for c in doc2.chunks if c.is_summary] + assert len(schunks) == 1 + assert schunks[0].position == -1 and schunks[0].embedding is not None + assert schunks[0].content == doc2.summary + finally: + _cleanup_source(db, root.name) + + +def test_changed_image_deletes_the_stale_copy_before_replacing( + db, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A CHANGED image (hash differs) deletes the stale ``image_path`` + copy before writing the new one — here with a MOVED ``image_dir`` + (the stale copy lives in the old dir and must be removed), the new + copy takes the same ``.png`` name in the new dir, and the + row's ``image_path``/``content`` follow.""" + root = tmp_path / "chgimg" + root.mkdir() + old_dir, new_dir = tmp_path / "img-old", tmp_path / "img-new" + v1, v2 = PNG_1X1, PNG_1X1 + b"replaced" # different bytes → different digest + + handed: list[bytes] = [] + state = {"desc": "v1 description"} + + async def _fake( + llm: importer.Embedder, *, data: bytes, source: str, rel: str, full_path: Path + ) -> str | None: + handed.append(data) + return state["desc"] + + monkeypatch.setattr(importer, "_describe_or_skip", _fake) + try: + (root / "pic.png").write_bytes(v1) + llm_old = _image_llm(tmp_path, image_dir=str(old_dir)) + s1 = asyncio.run(import_sources([root], llm_old, session=db)) + assert s1.added == 1 + doc = _doc(db, root.name, "pic.png") + assert doc.image_path is not None + old_copy = Path(doc.image_path) + assert old_copy.parent == old_dir and old_copy.exists() + + (root / "pic.png").write_bytes(v2) + state["desc"] = "v2 description" + llm_new = _image_llm(tmp_path, image_dir=str(new_dir)) + s2 = asyncio.run(import_sources([root], llm_new, session=db)) + assert (s2.updated, s2.added, s2.images_failed) == (1, 0, 0) + assert handed == [v1, v2] + + doc2 = _doc(db, root.name, "pic.png") + assert doc2.content == "v2 description" + assert doc2.image_path is not None + new_copy = Path(doc2.image_path) + assert new_copy.parent == new_dir and new_copy.name == old_copy.name + assert new_copy.read_bytes() == v2 + assert not old_copy.exists(), "the stale copy must be deleted" + finally: + _cleanup_source(db, root.name) + + +# --- the prune guard (LOCKED, derived from A3/A4) --------------------------- + + +def test_prune_guard_images_off_keeps_image_docs_and_on_prunes_them( + db, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The phase-122 prune guard: with the toggle OFF the walk is blind + to image files, and a ``prune=True`` run MUST NOT delete the + pre-existing image doc (invisible to the walk ≠ deleted) — its + copy survives too. With the toggle ON and the file gone, the + normal prune runs: doc + copy deleted.""" + root = tmp_path / "guardimg" + root.mkdir() + (root / "pic.png").write_bytes(PNG_1X1) + llm_on = _image_llm(tmp_path) + _patch_description(monkeypatch, "A red square.") + try: + s_on = asyncio.run(import_sources([root], llm_on, session=db)) + assert s_on.added == 1 + doc = _doc(db, root.name, "pic.png") + assert doc.image_path is not None + copy = Path(doc.image_path) + assert copy.exists() + + # Toggle OFF, file still on disk (but invisible to the walk): + # prune must NOT touch the image doc or its copy. + llm_off = _image_llm(tmp_path, images=False) + s_off = asyncio.run(import_sources([root], llm_off, session=db, prune=True)) + assert (s_off.files, s_off.pruned) == (0, 0) + _doc(db, root.name, "pic.png") # the doc survives + assert copy.exists(), "the copy survives with the doc" + + # Toggle ON, file GONE: normal prune — doc + copy deleted. + (root / "pic.png").unlink() + s_on2 = asyncio.run(import_sources([root], llm_on, session=db, prune=True)) + assert (s_on2.pruned, s_on2.added) == (1, 0) + assert ( + db.scalar(select(Document).where(Document.source == root.name)) is None + ) + assert not copy.exists(), "the copy is deleted with the doc" + finally: + _cleanup_source(db, root.name) + + +# --------------------------------------------------------------------------- +# Phase 122, task 03 — the description: the vision model writes the only +# embedded text (``describe_image`` + the filled seam + the image-aware +# summary storage). +# --------------------------------------------------------------------------- + + +class _VisionMock: + """Duck-typed chat-model mock for :func:`describe_image` (task 03): + records the message + model and returns a canned reply (or raises + it). Satisfies the summarizer's ``SummaryLLM`` protocol. + """ + + def __init__( + self, reply: str | Exception = "A red square on a white background." + ) -> None: + self.settings = _settings() + self.reply = reply + self.messages: list[Any] = [] + self.model: str | None = None + + async def chat( + self, messages: list[dict[str, Any]], model: str | None = None + ) -> str: + self.messages = list(messages) + self.model = model + if isinstance(self.reply, Exception): + raise self.reply + return self.reply + + +# --- describe_image: prompt shape, model, cap, fail-soft ------------------ + + +def test_describe_prompt_carries_the_mode_marker() -> None: + """The ``IMAGE_DESCRIPTION_MODE`` marker heads the describe prompt + (the ``SUMMARY_MODE``/``DEFLECT_MODE`` convention, PLAN §6 — the + deterministic E2E mock LLM keys on it; the story suite wires the + mock's branch in task 06). Pinned here: a re-wrap of the prompt + cannot silently re-route the mock, and the retrieval-oriented + instruction stays in the constant.""" + assert DESCRIBE_PROMPT.startswith(IMAGE_DESCRIPTION_MODE + ": ") + for fragment in ( + "faithfully", + "visible text", + "diagram, table", + "salient details", + "2-4 sentences", + "ONLY text", + ): + assert fragment in DESCRIBE_PROMPT + + +def test_describe_image_prompt_shape_multimodal_and_chat_model() -> None: + """ONE chat-model call (LOCKED A3 — the vision ``llm_chat_model``, + NOT the ``lite`` summary model) with the OpenAI-compatible + multimodal user message: the fixed prompt's text part + the + ``image_url`` part whose data URL is ``data:;base64,`` + — no system prompt, no tools.""" + mock = _VisionMock("A red square on a white background.") + result = asyncio.run(describe_image(mock, data=PNG_1X1, mime="image/png")) + assert result == "A red square on a white background." + assert len(mock.messages) == 1 + message = mock.messages[0] + assert message["role"] == "user" + content = message["content"] + assert isinstance(content, list) and len(content) == 2 + assert content[0] == {"type": "text", "text": DESCRIBE_PROMPT} + b64 = base64.b64encode(PNG_1X1).decode("ascii") + assert content[1] == { + "type": "image_url", + "image_url": {"url": f"data:image/png;base64,{b64}"}, + } + # LOCKED A3: the CHAT (vision) model — never the ``lite`` summary + # model (it is not assumed vision-capable). + assert mock.model == mock.settings.llm_chat_model + assert mock.model != mock.settings.llm_summary_model + + +def test_describe_image_caps_at_summary_max_chars() -> None: + """The reply is cut exactly at ``summary_max_chars`` (the phase-30 + cap — the description IS the summary, so it keeps the same uniform + ceiling). The explicit ``settings`` parameter wins; ``settings=None`` + falls back to the LLM's own settings.""" + long_reply = "x" * 500 + mock = _VisionMock(long_reply) + capped = asyncio.run( + describe_image( + mock, + data=PNG_1X1, + mime="image/png", + settings=_settings(summary_max_chars=100), + ) + ) + assert capped == "x" * 100 + mock2 = _VisionMock(long_reply) + mock2.settings = _settings(summary_max_chars=42) + assert ( + asyncio.run(describe_image(mock2, data=PNG_1X1, mime="image/png")) + == "x" * 42 + ) + + +def test_describe_image_strips_and_returns_none_on_failure_classes() -> None: + """The reply is whitespace-stripped; ``None`` (the caller's + fail-soft skip) on a client error, an empty reply, or a + whitespace-only reply.""" + mock = _VisionMock(" A red square.\n") + assert ( + asyncio.run(describe_image(mock, data=PNG_1X1, mime="image/png")) + == "A red square." + ) + failing = _VisionMock(LLMError("simulated vision-model failure (test sentinel)")) + assert asyncio.run(describe_image(failing, data=PNG_1X1, mime="image/png")) is None + for empty in ("", " \n"): + blank = _VisionMock(empty) + assert ( + asyncio.run(describe_image(blank, data=PNG_1X1, mime="image/png")) + is None + ) + + +def test_image_mimes_cover_the_default_family() -> None: + """The extension → MIME map (task 03; task 04 reuses it for the + serve route's ``Content-Type`` — one map, one truth) covers the + default image family — dotted lowercase keys, the + ``image_extension_set`` shape — and the unlisted-token fallback is + the generic mime (a custom ``BOR_IMAGE_EXTENSIONS`` format fails + soft at the vision endpoint, never a guessed type).""" + assert IMAGE_MIMES == { + ".png": "image/png", + ".jpg": "image/jpeg", + ".jpeg": "image/jpeg", + ".webp": "image/webp", + ".gif": "image/gif", + ".bmp": "image/bmp", + } + assert IMAGE_FALLBACK_MIME == "application/octet-stream" + assert all(ext in IMAGE_MIMES for ext in _settings().image_extension_set) + + +# --- the REAL seam (no patch): description → content = summary = the only +# embedded text; failure → skip; changed + failure → old row stays ------- + + +def test_image_description_real_seam_content_summary_and_only_embedded_text( + db, tmp_path: Path +) -> None: + """The REAL seam (no patch): the vision description becomes BOTH + ``content`` and ``summary`` (the ``is_summary`` position −1 chunk + mirrors it), the describe call went to the CHAT model (LOCKED + A3), and the ONLY text ever handed to the embedder for the image is + that description — the embedding model never sees pixels. The text + doc in the same source is summarized by the ``lite`` stand-in, + untouched by the image machinery.""" + root = tmp_path / "realseam" + root.mkdir() + (root / "pic.png").write_bytes(PNG_1X1) + (root / "notes.md").write_text("# Notes\n\nBody.\n", encoding="utf-8") + llm = _image_llm(tmp_path, _VisionMockLLM) + try: + summary = asyncio.run(import_sources([root], llm, session=db)) + assert (summary.files, summary.added, summary.images_failed) == (2, 2, 0) + assert summary.summaries == 2 # the text doc + the image doc + # The walk is sorted: notes.md first (its ``lite`` summary), + # then pic.png (the multimodal describe — the CHAT model). + assert llm.chat_models == [ + llm.settings.llm_summary_model, + llm.settings.llm_chat_model, + ] + + doc = _doc(db, root.name, "pic.png") + assert doc.is_image is True + assert doc.content == _VisionMockLLM.DESCRIPTION + assert doc.summary == _VisionMockLLM.DESCRIPTION # task 03: verbatim + chunks = db.scalars(select(Chunk).where(Chunk.document_id == doc.id)).all() + assert all(c.content == _VisionMockLLM.DESCRIPTION for c in chunks) + summary_chunks = [c for c in chunks if c.is_summary] + assert len(summary_chunks) == 1 and summary_chunks[0].position == -1 + + # The ONLY embedded text of the image doc is the description — + # twice (the one content chunk + the is_summary chunk) — and no + # base64/bytes anywhere in what the embedder saw. + embedded = [t for batch in llm.calls for t in batch] + assert embedded.count(_VisionMockLLM.DESCRIPTION) == 2 + assert len(embedded) == 4 + assert not any("iVBORw0KGgo" in t for t in embedded) + finally: + _cleanup_source(db, root.name) + + +def test_image_description_real_seam_failure_skips_and_keeps_sync_green( + db, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """The REAL seam with a NON-VISION chat model (LOCKED A3's honest + failure — the SDK errors, ``describe_image`` returns ``None``): + the doc is skipped (no row, no copy, not even the dir), + ``images_failed`` is bumped, the warning is logged, and the sync + completes with the other docs indexed.""" + root = tmp_path / "realfail" + root.mkdir() + (root / "pic.png").write_bytes(PNG_1X1) + (root / "notes.md").write_text("# Notes\n\nBody.\n", encoding="utf-8") + llm = _image_llm(tmp_path, _VisionFailsLLM) + try: + with caplog.at_level(logging.INFO, logger="app.importer"): + summary = asyncio.run(import_sources([root], llm, session=db)) + finally: + _cleanup_source(db, root.name) + + assert (summary.files, summary.added, summary.images_failed) == (2, 1, 1) + assert ( + db.scalar( + select(Document).where( + Document.source == root.name, Document.path == "pic.png" + ) + ) + is None + ), "a failed description must leave NO row" + assert not Path(llm.settings.image_dir).expanduser().exists() + # The importer's line names the document (the PLAN §9-style signal); + # the seam's line names the REASON (model + the client error) — + # together, the visible, greppable failure. + doc_warnings = [ + r + for r in caplog.records + if r.name == "app.importer" and "image description failed" in r.getMessage() + ] + assert len(doc_warnings) == 1 + assert doc_warnings[0].levelno == logging.WARNING + assert "source=realfail path=pic.png" in doc_warnings[0].getMessage() + reason_warnings = [ + r + for r in caplog.records + if r.name == "app.summarizer" and "image description failed" in r.getMessage() + ] + assert len(reason_warnings) == 1 + assert "model=turbo" in reason_warnings[0].getMessage() + + +def test_changed_image_with_failed_description_keeps_the_old_row( + db, tmp_path: Path +) -> None: + """A CHANGED image whose new bytes fail to describe: the doc is + skipped (fail-soft) — the OLD row stays as-is (content, hash, and + copy unmutated — the skip happens before any write), + ``images_failed`` is bumped, and the sync completes.""" + root = tmp_path / "keeprow" + root.mkdir() + v1, v2 = PNG_1X1, PNG_1X1 + b"changed" + (root / "pic.png").write_bytes(v1) + llm_ok = _image_llm(tmp_path, _VisionMockLLM) + try: + s1 = asyncio.run(import_sources([root], llm_ok, session=db)) + assert s1.added == 1 + doc1 = _doc(db, root.name, "pic.png") + old_content, old_hash = doc1.content, doc1.content_hash + assert doc1.image_path is not None + old_copy = Path(doc1.image_path) + assert old_copy.exists() + + (root / "pic.png").write_bytes(v2) + llm_fail = _image_llm(tmp_path, _VisionFailsLLM) + s2 = asyncio.run(import_sources([root], llm_fail, session=db)) + assert (s2.files, s2.added, s2.updated, s2.images_failed) == (1, 0, 0, 1) + + doc2 = _doc(db, root.name, "pic.png") + assert (doc2.content, doc2.content_hash) == (old_content, old_hash) + assert doc2.image_path is not None + assert Path(doc2.image_path) == old_copy and old_copy.exists() + finally: + _cleanup_source(db, root.name) + + +# --------------------------------------------------------------------------- +# Phase 122, task 04 — serve + display: the content endpoint's wire +# affordance, the tree node's omission rule, and the frontend source +# contracts (house-style source assertions). +# --------------------------------------------------------------------------- + +from app.schemas import DocContent, KbTreeFile # noqa: E402 + +FRONTEND = Path(__file__).resolve().parents[2] / "frontend" +DOCUMENT_JS = FRONTEND / "assets" / "document.js" +SOURCES_JS = FRONTEND / "assets" / "sources.js" +STYLES_CSS = FRONTEND / "assets" / "styles.css" + + +def _text_doc_content() -> DocContent: + return DocContent( + source="S", + path="a.md", + title="A", + format="md", + created_at="2026-01-01T00:00:00+00:00", + content="body", + indexed_at="2026-01-01T00:00:00+00:00", + chunks=1, + ) + + +_UNSET: Any = object() + + +def _image_doc_content(doc_id: str = "d" * 32, image_url: Any = _UNSET) -> DocContent: + """*image_url* defaults to the bytes route's path for *doc_id*; + pass an explicit ``None`` for the row-without-a-copy case (the + sentinel keeps ``None`` meaningful).""" + return DocContent( + source="S", + path="pic.png", + title="pic", + format="png", + summary="A red square.", + created_at="2026-01-01T00:00:00+00:00", + content="A red square.", + indexed_at="2026-01-01T00:00:00+00:00", + chunks=2, + is_image=True, + image_url=f"/api/documents/{doc_id}/image" if image_url is _UNSET else image_url, + ) + + +def test_doc_content_text_doc_wire_shape() -> None: + """Task 04 wire contract: a TEXT doc response gains exactly ONE new + key — ``is_image: false`` — and ``image_url`` is ABSENT (never + null, the ``_drop_absent_share_url`` omission precedent). The new + keys introduce no nulls anywhere in the text-doc shape (the + pre-existing ``summary: null`` — the phase-36 markdown contract — + is the only null; the integration suite pins the endpoint level, + this pins the model level).""" + dumped = _text_doc_content().model_dump() + assert dumped["is_image"] is False # always present, never null + assert "image_url" not in dumped # absent — never null + + +def test_doc_content_image_doc_wire_shape() -> None: + """An IMAGE doc response carries ``is_image: true`` + ``image_url`` + (the bytes route's path). A ``None`` image_url (the row-without-a- + copy corner) is omitted, not null — the viewer's onerror fallback + covers it.""" + dumped = _image_doc_content(doc_id="1" * 32).model_dump() + assert dumped["is_image"] is True + assert dumped["image_url"] == "/api/documents/" + "1" * 32 + "/image" + missing_copy = _image_doc_content(image_url=None).model_dump() + assert missing_copy["is_image"] is True + assert "image_url" not in missing_copy # absent — never null + + +def test_kb_tree_file_text_node_omits_the_image_affordance() -> None: + """The tree's file-node omission rule (the wire contract behind the + phase's byte-identical criterion): a text file node serializes + byte-identically to pre-phase (all three image keys absent — not + false/null), an image node keeps ``is_image`` + ``summary`` and + drops a ``None`` ``image_url``.""" + text = KbTreeFile(path="a.md", title="A", chunks=1, created_at="x", indexed_at="y") + assert set(text.model_dump()) == { + "kind", "path", "title", "chunks", "created_at", "indexed_at" + } + image = KbTreeFile( + path="pic.png", + title="pic", + chunks=2, + created_at="x", + indexed_at="y", + is_image=True, + image_url="/api/documents/abc/image", + summary="A red square.", + ) + dumped = image.model_dump() + assert dumped["is_image"] is True + assert dumped["image_url"] == "/api/documents/abc/image" + assert dumped["summary"] == "A red square." + no_copy = KbTreeFile( + path="pic.png", title="pic", chunks=2, created_at="x", indexed_at="y", is_image=True + ) + dumped = no_copy.model_dump() + assert dumped["is_image"] is True + assert "image_url" not in dumped # absent — the glyph fallback renders + assert dumped["summary"] is None # null stays (the alt fallback corner) + + +def test_viewer_renders_the_image_block_from_image_url_with_alt_summary() -> None: + """House-style source contract (document.js, the ONE shared + renderDocument core — page + modal): an ``is_image`` doc renders + the ```` block from ``doc.image_url`` (properties only — src / + alt, never innerHTML with document-derived data), with ``alt`` = + the summary (the WCAG alt contract — a NULL/blank summary falls + back to the title), and the onerror fallback swaps in the + "Image unavailable" note (the route's 404 — the row exists but the + copy was lost; the description below still renders).""" + js = DOCUMENT_JS.read_text(encoding="utf-8") + assert "if (doc.is_image) {" in js, "the image block must gate on doc.is_image" + assert 'wrap.className = "doc-image"' in js + assert "img.src = doc.image_url" in js, "the must come from doc.image_url" + # alt = the summary (the vision description) with the title fallback. + assert "img.alt" in js + assert "doc.summary.trim() !== \"\"" in js + assert "? doc.summary" in js + assert ": doc.title" in js + # The onerror fallback — a small note (role=status), not a broken img. + assert 'img.addEventListener("error"' in js + assert 'note.className = "doc-image-unavailable"' in js + assert 'note.setAttribute("role", "status")' in js + assert "Image unavailable" in js + # The description stays in the normal content slot below (the + # existing plain-content path — doc-raw via textContent). + assert "pre.textContent = doc.content" in js + # ONE innerHTML in the module CODE (the XSS contract, unchanged — + # the escape-first markdown render; comments may name it, the + # count pin reads code only). + code = re.sub(r"//.*", "", re.sub(r"/\*.*?\*/", "", js, flags=re.S)) + assert code.count("innerHTML") == 1 + assert "wrap.innerHTML = renderMarkdown(doc.content)" in code + + +def test_viewer_suppresses_the_duplicate_summary_panel_for_images() -> None: + """Phase 122 (task 04): for an image doc whose summary IS the + verbatim description (summary === content — the importer + invariant), the labeled Summary panel would duplicate the text + right below the image, so the panel is suppressed; an admin-edited + summary (different text) still renders with the phase-57 + affordance (the gate stays a summary-presence check, only widened + by the image-verbatim exclusion).""" + js = DOCUMENT_JS.read_text(encoding="utf-8") + assert "doc.is_image && doc.summary === doc.content" in js + assert "if (doc.summary && doc.summary.trim() !== \"\" && !imageSummaryIsContent) {" in js + + +def test_sources_row_thumbnail_falls_back_to_the_glyph_on_error() -> None: + """House-style source contract (sources.js makeRow): an image row + gets the FIXED 48px thumbnail box before the path link (the + .kb-doc-path flex wrapper — the link keeps its ellipsis), with + ``loading="lazy"`` and ``alt = summary`` (title/path fallback); a + failed fetch swaps in the document glyph INSIDE the same box + (progressive enhancement — no layout shift, no broken-image + placeholder). Text rows keep the bare-link cell (the ``d.is_image`` + gate). The glyph is static markup (aria-hidden; the module keeps + its ONE innerHTML — the sync modal).""" + js = SOURCES_JS.read_text(encoding="utf-8") + assert 'pathWrap.className = "kb-doc-path"' in js + assert "if (d.is_image) {" in js, "only image rows get the box" + assert 'box.className = "kb-doc-thumb"' in js + assert 'img.loading = "lazy"' in js + assert "img.src = d.image_url" in js + # alt = the summary, with the title/path fallback. + assert "img.alt = alt" in js + assert "d.summary.trim() !== \"\"" in js + # The onerror fallback — the glyph IN the same fixed box. + assert 'img.addEventListener("error"' in js + assert "box.replaceChildren(docThumbGlyph())" in js + assert 'span.className = "kb-doc-thumb-glyph"' in js + assert 'span.setAttribute("aria-hidden", "true")' in js + # The module's ONE innerHTML is still the sync-modal skeleton. + code = re.sub(r"//.*", "", re.sub(r"/\*.*?\*/", "", js, flags=re.S)) + assert code.count("innerHTML") == 1 + # The fixed box is 48px in the CSS (object-fit: cover, the lazy + # img fills it). + css = STYLES_CSS.read_text(encoding="utf-8") + assert "width: 48px" in css + assert "object-fit: cover" in css + + +def test_serve_css_covers_viewer_image_and_sources_thumb() -> None: + """The phase-122 CSS ships: the viewer's image block (the theme's + surface treatment, max-width 100% on the img, the reading-column + cap — test_wide_column_css pins the token count) + the unavailable + note, and the Sources thumbnail box (border/radius on theme + tokens).""" + css = STYLES_CSS.read_text(encoding="utf-8") + for selector in (".doc-image", ".doc-image-img", ".doc-image-unavailable", + ".kb-doc-thumb", ".kb-doc-thumb-img", ".kb-doc-thumb-glyph"): + assert re.search(rf"^{re.escape(selector)} \{{", css, re.M), f"missing rule: {selector}" + assert "max-width: 100%" in css + + +# --------------------------------------------------------------------------- +# Phase 122, task 05 — the RAG display: the SSE source frames' optional +# ``image_url`` (the shared ``source_ref_with_image`` builder), the +# agent ``read`` marker, and the chat sources block's inline image +# (house-style frontend source contracts; the SSE-level byte pin sits in +# tests/integration/test_chat_api.py, the E2E rendering in task 06's +# isolated story suite). +# --------------------------------------------------------------------------- + +import uuid # noqa: E402 +from datetime import UTC, datetime # noqa: E402 + +from app.rag.agent import ( # noqa: E402 + IMAGE_DOC_MARKER, + READ_TRUNCATION_NOTICE, + AgentHolder, + _execute_tool, +) +from app.rag.llm import ToolCallPiece # noqa: E402 +from app.rag.retriever import TRUNCATION_MARKER, source_ref_with_image # noqa: E402 +from app.schemas import ChatDoneEvent, SourceRef # noqa: E402 + +APP_JS = FRONTEND / "assets" / "app.js" + + +def _function_body(js: str, header: str) -> str: + """The full text of the function whose header is ``header`` — from + the header to its brace-matched closing ``}`` (the + test_source_chip_quality house pin; the pinned functions' template + literals carry balanced ``${…}`` pairs and no string literal holds + a stray brace).""" + start = js.index(header) + body_open = js.index("{", start) + depth = 0 + for j in range(body_open, len(js)): + if js[j] == "{": + depth += 1 + elif js[j] == "}": + depth -= 1 + if depth == 0: + return js[start : j + 1] + raise AssertionError(f"unbalanced braces in {header!r}") + + +def _frame_doc(is_image: bool) -> Document: + """A detached documents row for the frame-helper pins (never + persisted — ``source_ref_with_image`` only reads identity fields). + ``is_image`` rows also carry the ``image_path`` the importer would + have stored (the frame never uses it — the id alone builds the + route).""" + path = "pic.png" if is_image else "a.md" + return Document( + id=uuid.uuid4(), + source="docs", + path=path, + full_path=f"/docs/{path}", + title="pic" if is_image else "A", + content="A red square." if is_image else "body", + content_hash="0" * 64, + created_at=datetime(2024, 6, 15, 12, 0, 0, tzinfo=UTC), + is_image=is_image, + image_path=None if not is_image else "/tmp/img.png", + ) + + +# --- the shared frame builder: image_url present/absent (omission rule) + + +def test_source_ref_with_image_carries_the_bytes_route_for_image_docs() -> None: + """The shared builder (``app.rag.retriever.source_ref_with_image`` — + the chat API's cited AND related tiers both run through it): an + ``is_image`` doc's ref carries the OPTIONAL ``image_url`` — the + bytes route ``/api/documents//image`` (task 04's route, the + same str-uuid form the content endpoint builds).""" + doc = _frame_doc(is_image=True) + ref = source_ref_with_image(doc) + assert (ref.source, ref.path, ref.title) == ("docs", "pic.png", "pic") + assert ref.image_url == f"/api/documents/{doc.id}/image" + dumped = ref.model_dump() + assert dumped["image_url"] == f"/api/documents/{doc.id}/image" # rides the wire + + +def test_source_ref_with_image_text_doc_frame_is_byte_identical() -> None: + """The omission rule: a TEXT doc's ref has ``image_url`` ``None`` + and the model's serializer DROPS the key (never ``null``) — the + wire key set stays exactly the pre-phase-122 shape, so a text-doc + frame is byte-identical to before.""" + doc = _frame_doc(is_image=False) + ref = source_ref_with_image(doc) + assert ref.image_url is None + dumped = ref.model_dump() + assert set(dumped) == {"source", "path", "title"} # no image_url key + assert "image_url" not in dumped + + +def test_done_frame_carries_image_url_only_on_the_image_ref() -> None: + """Nested-serializer pin: inside the SSE ``done`` frame (the chat + bubble's render input), the text refs keep the pre-122 key set and + ONLY the image ref carries ``image_url`` (the frame's only new + field — no other key, no doc-id leak beyond the path). A pre-phase + client-saved ref (no ``image_url`` key) still parses — the ``None`` + default keeps old saved chats intact.""" + image, text = _frame_doc(is_image=True), _frame_doc(is_image=False) + done = ChatDoneEvent( + deflected=False, + sources=[source_ref_with_image(text), source_ref_with_image(image)], + related=[source_ref_with_image(text)], + ) + dumped = done.model_dump() + assert set(dumped["sources"][0]) == {"source", "path", "title"} # text: pre-122 + assert dumped["sources"][1]["image_url"] == f"/api/documents/{image.id}/image" + assert "image_url" not in dumped["related"][0] # text: omitted, never null + old_saved = SourceRef.model_validate( + {"source": "docs", "path": "a.md", "title": "A"} + ) + assert old_saved.image_url is None # pre-phase saved refs parse + + +# --- the agent read marker: image docs read as descriptions + + +def test_image_doc_marker_is_the_pinned_line() -> None: + """The marker is a module constant (``agent.IMAGE_DOC_MARKER``) — + the model must reason about what it is reading: the text is a + description GENERATED from the image, not the image's own words.""" + assert IMAGE_DOC_MARKER == ( + "Image document — the text below is a description generated from the image:" + ) + + +def _seed_read_doc(db, *, is_image: bool, content: str) -> Document: + """One real documents row the ``read`` tool can resolve (source + ``imgsrc`` — no fixture collision); cleaned up by the caller.""" + path = "pic.png" if is_image else "a.md" + doc = Document( + id=uuid.uuid4(), + source="imgsrc", + path=path, + full_path=f"/imgsrc/{path}", + title="pic" if is_image else "A", + content=content, + content_hash="0" * 64, + created_at=datetime(2024, 6, 15, 12, 0, 0, tzinfo=UTC), + is_image=is_image, + ) + if is_image: + doc.image_path = f"/tmp/{doc.id}.png" + db.add(doc) + db.commit() + return doc + + +def _read(db, settings, combined: str) -> tuple[str, AgentHolder]: + holder = AgentHolder() + result = _execute_tool( + db, + ToolCallPiece(id="call_1", name="read", arguments={"path": combined}), + [], + holder, + settings, + ) + return result, holder + + +def test_read_of_image_doc_prefaces_the_description_with_the_marker(db) -> None: + """The ``read`` result for an image doc: the ``Document …`` header + and the phase-106 date line stay BYTE-IDENTICAL (the E2E mock's + ``_READ_RESULT_PREFIX`` header contract), and the marker line sits + between the date line and the description — the model sees the + description (the doc's ``content``), never raw bytes.""" + doc = _seed_read_doc( + db, is_image=True, content="A red square on a white background." + ) + try: + result, holder = _read(db, _settings(), "imgsrc/pic.png") + assert result == ( + "Document imgsrc/pic.png:\n" + "date: 2024-06-15\n" + f"{IMAGE_DOC_MARKER}\n" + "A red square on a white background." + ) + assert holder.read_docs == [doc] # an image read IS a citation read + assert holder.tool_calls == 1 + finally: + db.delete(doc) + db.commit() + + +def test_read_of_text_doc_result_stays_byte_identical(db) -> None: + """A non-image doc's ``read`` result carries NO marker — + byte-identical to the pre-phase-122 result (header + date line + + content, nothing else).""" + doc = _seed_read_doc(db, is_image=False, content="CONTENT") + try: + result, _holder = _read(db, _settings(), "imgsrc/a.md") + assert result == "Document imgsrc/a.md:\ndate: 2024-06-15\nCONTENT" + assert IMAGE_DOC_MARKER not in result + finally: + db.delete(doc) + db.commit() + + +def test_read_of_image_doc_over_the_cap_truncates_after_the_marker(db) -> None: + """The phase-95 cap still applies to image docs (the description + is long prose like any content): the marker precedes the CUT, the + shared truncation marker + notice follow the cut — and a text doc + of the same length truncates WITHOUT the marker (byte-identical + to pre-122).""" + content = "x" * 50 + image = _seed_read_doc(db, is_image=True, content=content) + text = _seed_read_doc(db, is_image=False, content=content) + settings = _settings(read_max_chars=10) + try: + img_result, _ = _read(db, settings, "imgsrc/pic.png") + assert img_result == ( + "Document imgsrc/pic.png:\n" + "date: 2024-06-15\n" + f"{IMAGE_DOC_MARKER}\n" + f"{'x' * 10}\n" + f"{TRUNCATION_MARKER}\n" + f"{READ_TRUNCATION_NOTICE.format(shown=10, total=50)}" + ) + txt_result, _ = _read(db, settings, "imgsrc/a.md") + assert txt_result == ( + "Document imgsrc/a.md:\n" + "date: 2024-06-15\n" + f"{'x' * 10}\n" + f"{TRUNCATION_MARKER}\n" + f"{READ_TRUNCATION_NOTICE.format(shown=10, total=50)}" + ) + assert IMAGE_DOC_MARKER not in txt_result + finally: + db.delete(image) + db.delete(text) + db.commit() + + +# --- the chat sources block: the inline image (house-style frontend) + + +def test_chat_sources_figure_reads_image_url_sets_alt_and_collapses_on_error() -> None: + """House-style source contract (app.js appendSourceImageFigure — + task 05's "shown in the chat nicely" renderer): the figure's + ```` comes from the ref's ``image_url``; ``alt`` is set + (title first, then the SUMMARY once the content fetch settles — + the frame carries no summary, image_url is its only new field) + and so is the VISIBLE caption; a FAILED image load removes the + whole figure — it collapses to the plain chip, never a + broken-image icon. Properties only (src/alt/textContent) — the + figure builds no innerHTML from document-derived data.""" + js = APP_JS.read_text(encoding="utf-8") + body = _function_body(js, "function appendSourceImageFigure(meta, s) {") + assert "img.src = s.image_url" in body, "the must come from s.image_url" + assert "img.alt = s.title || label" in body, "alt is set from the start (title)" + assert "img.alt = summary" in body, "alt becomes the summary when it resolves" + assert "caption.textContent = s.title || label" in body # visible caption: title + assert "caption.textContent = summary" in body # …then the summary + assert 'img.addEventListener("error"' in body, "the error handler must exist" + assert "fig.remove();" in body, "a failed load collapses to the plain chip" + assert "fetchContentSummary(s.source, s.path)" in body, ( + "the frame carries no summary — the figure fetches it" + ) + code = re.sub(r"//.*", "", re.sub(r"/\*.*?\*/", "", body, flags=re.S)) + assert "innerHTML" not in code, "no innerHTML from document-derived data" + # The figure reuses the chip's navigation (phase 26 contract): + assert 'fig.href = documentUrl(s.source, s.path, "/");' in body + assert "openDocumentModal(s.source, s.path, fig);" in body + + +def test_chat_sources_and_related_rows_gate_the_figure_on_image_url() -> None: + """Both chat source surfaces (the citation chips AND the related + row — the frame's two ref tiers, built by the same server helper) + render the figure ONLY when the ref carries ``image_url``: text + refs (and every pre-phase saved ref) add zero new DOM — the + renderer stays byte-identical for them. The chip's text + + affordance stay (the figure is additive, not a replacement), and + the summary fetch targets the SAME content endpoint the chip's + modal already uses (failing soft to null — the title stands).""" + js = APP_JS.read_text(encoding="utf-8") + src = _function_body(js, "function appendSources(wrap, sources) {") + assert 'if (s.image_url) appendSourceImageFigure(meta, s);' in src + assert 'chip.className = "source-chip";' in src, "the chip stays (additive)" + assert "chip.textContent = label" in src + rel = _function_body(js, "function appendRelated(wrap, related) {") + assert 'if (s.image_url) appendSourceImageFigure(row, s);' in rel + fetch = _function_body(js, "function fetchContentSummary(source, path) {") + assert '"/api/documents/content?source="' in fetch + assert ".catch(() => null)" in fetch, "a failed fetch fails soft (null)" + + +def test_chat_sources_figure_css_is_compact_and_themed() -> None: + """The figure's CSS (theme tokens only — the zero-literal + invariant, so the monochrome themes carry it): a COMPACT img + (max-height 96px, object-fit: contain) on the theme's surface, + the caption in the AA-safe muted ink, and the hover is the flat + underline (the related-doc pattern — no background swap, no + elevation).""" + css = STYLES_CSS.read_text(encoding="utf-8") + for selector in (".source-image", ".source-image-img", ".source-image-caption"): + assert re.search(rf"^{re.escape(selector)} \{{", css, re.M), f"missing rule: {selector}" + img = re.search(r"^\.source-image-img \{\n([\s\S]*?)\n\}", css, re.M) + assert img, "missing .source-image-img rule" + img_body = img.group(1) + assert "max-height: 96px" in img_body # the compact cap (the task's ~96px) + assert "object-fit: contain" in img_body + assert "background: var(--surface)" in img_body # the theme's surface + cap = re.search(r"^\.source-image-caption \{\n([\s\S]*?)\n\}", css, re.M) + assert cap, "missing .source-image-caption rule" + assert "color: var(--ink-soft)" in cap.group(1) # AA: 5.1:1 on --surface + hover = re.search(r"^\.source-image:hover \{([^}]*)\}", css, re.M) + assert hover and "text-decoration: underline" in hover.group(1), ( + "the hover is the flat underline (no background swap)" + ) + assert "background" not in (hover.group(1) if hover else "") diff --git a/tests/unit/test_importer.py b/tests/unit/test_importer.py index d664087..d872715 100644 --- a/tests/unit/test_importer.py +++ b/tests/unit/test_importer.py @@ -970,12 +970,14 @@ def test_import_summary_log_line_includes_summary_counters( """PLAN §9 summary line: the phase-30 counters sit between ``embed_batches`` and ``formats``; the phase-118 backfill counter sits between ``summary_errors`` and ``dates_updated``; the - phase-106 date-refresh counter sits before ``formats``.""" + phase-106 date-refresh counter and the phase-122 ``images_failed`` + counter sit after ``dates_updated``, before ``formats``.""" s = ImportSummary() s.files, s.added, s.chunks, s.embed_batches = 3, 3, 5, 4 s.summaries, s.summary_errors = 2, 1 s.summary_backfilled = 1 s.dates_updated = 0 + s.images_failed = 0 s.formats = {"md": 1, "yaml": 2} with caplog.at_level(logging.INFO, logger="app.importer"): s.log() @@ -983,7 +985,7 @@ def test_import_summary_log_line_includes_summary_counters( assert line == ( "import: summary files=3 added=3 updated=0 unchanged=0 pruned=0 errors=0 " "chunks=5 embed_batches=4 summaries=2 summary_errors=1 summary_backfilled=1 " - "dates_updated=0 formats=yaml:2,md:1" + "dates_updated=0 images_failed=0 formats=yaml:2,md:1" ) @@ -1103,13 +1105,17 @@ def test_no_progress_means_no_prewalk( excluded: frozenset[str] = EXCLUDED_DIRS, ignore: tuple[str, ...] = (), include_hidden: bool = False, + image_extensions: frozenset[str] = frozenset(), ) -> list[Path]: # Phase 89: the walker gained the ``ignore`` keyword; phase 105: - # the ``include_hidden`` flag — the sentinel accepts (and forwards) - # both to stay a drop-in. + # the ``include_hidden`` flag; phase 122: the ``image_extensions`` + # set — the sentinel accepts (and forwards) all three to stay a + # drop-in. nonlocal walk_calls walk_calls += 1 - return real_walker(r, extensions, excluded, ignore, include_hidden) + return real_walker( + r, extensions, excluded, ignore, include_hidden, image_extensions + ) monkeypatch.setattr(importer, "iter_importable_files", counting) try: diff --git a/tests/unit/test_kb_tree_builder.py b/tests/unit/test_kb_tree_builder.py index b4a1a68..3c91dd1 100644 --- a/tests/unit/test_kb_tree_builder.py +++ b/tests/unit/test_kb_tree_builder.py @@ -169,14 +169,21 @@ def test_summaries_present_and_absent() -> None: one, two = _folder_nodes(source) assert one.summary == "One desc." assert two.summary is None - # File nodes carry no summary key at all (the 00_phase.md shape); - # since phase 106 they DO carry the creation date (``created_at`` - # — the RAG view's ``Created`` column). + # File nodes carry no summary key at all on the WIRE (the + # 00_phase.md shape — the model_dump set check below is the wire + # pin); since phase 106 they DO carry the creation date + # (``created_at`` — the RAG view's ``Created`` column). file = _file_nodes(one)[0] assert set(file.model_dump()) == { "kind", "path", "title", "chunks", "created_at", "indexed_at" } - assert "summary" not in file.__class__.model_fields + # Phase 122 (task 04): the image-affordance fields DO exist on the + # class now (image file nodes set them — the wire omission for text + # nodes is the ``KbTreeFile`` serializer, pinned by the model_dump + # set check above: a text node never leaks the three keys). + from app.schemas import KbTreeFile + + assert {"is_image", "image_url", "summary"} <= set(KbTreeFile.model_fields) def test_file_metadata_unchanged_in_tree() -> None: @@ -546,3 +553,83 @@ def test_updated_at_does_not_leak_across_sources() -> None: a, b = build_kb_tree(["A", "B"], rows, {}) assert a.updated_at == C0 assert b.updated_at == C3 + + +# --------------------------------------------------------------------------- +# Phase 122 (task 04) — the image-docs affordance on tree file nodes +# --------------------------------------------------------------------------- + +DOC_ID = "11111111-2222-3333-4444-555555555555" + + +def test_image_file_node_carries_the_affordance_and_text_node_unchanged() -> None: + """The ``images`` map (``{(source, path): (doc_id, summary)}``) turns + a file node into the image-docs node: ``is_image`` true, + ``image_url`` = the bytes route's path built from the mapped id, + ``summary`` verbatim (the RAG view's thumbnail ``alt``). A file + node NOT in the map keeps the pre-phase wire shape byte-identically + (the three image keys are OMITTED, not false/null).""" + rows = [ + ("S", "one/a.md", "A", 1, T0, C0), + ("S", "one/pic.png", "pic", 2, T0, C0), + ] + images = {("S", "one/pic.png"): (DOC_ID, "A red square on a white background.")} + (source,) = build_kb_tree(["S"], rows, {}, images) + one = _folder_nodes(source)[0] + # File nodes keep the FULL source-relative path (the phase-97 + # shape — the folder prefix rides the node). + files = {f.path: f for f in _file_nodes(one)} + + # Text node: the pre-phase wire shape, byte-identical (no image keys). + assert set(files["one/a.md"].model_dump()) == { + "kind", "path", "title", "chunks", "created_at", "indexed_at" + } + + # Image node: the affordance rides the node. + dumped = files["one/pic.png"].model_dump() + assert dumped["kind"] == "file" + assert dumped["is_image"] is True + assert dumped["image_url"] == f"/api/documents/{DOC_ID}/image" + assert dumped["summary"] == "A red square on a white background." + # The catalogue fields still ride verbatim. + assert (dumped["title"], dumped["chunks"]) == ("pic", 2) + assert (dumped["created_at"], dumped["indexed_at"]) == (C0, T0) + + +def test_image_file_node_null_summary_and_missing_map_entry() -> None: + """A mapped image node with a NULL summary (the fail-soft backfill + corner) keeps ``summary: null`` on the wire (meaningful — the alt + falls back client-side). A map entry that points at a NON-existent + (source, path) affects nothing (the builder only reads mapped keys + it meets in the catalogue rows).""" + rows = [ + ("S", "one/ghost.png", "ghost", 1, T0, C0), + ("S", "one/other.md", "Other", 1, T0, C0), + ] + images = { + ("S", "one/ghost.png"): (DOC_ID, None), + ("S", "one/absent.png"): (DOC_ID, "never matched"), + } + (source,) = build_kb_tree(["S"], rows, {}, images) + one = _folder_nodes(source)[0] + files = {f.path: f for f in _file_nodes(one)} + ghost = files["one/ghost.png"].model_dump() + assert ghost["is_image"] is True + assert ghost["summary"] is None # null stays (the alt fallback corner) + assert ghost["image_url"] == f"/api/documents/{DOC_ID}/image" + assert set(files["one/other.md"].model_dump()) == { + "kind", "path", "title", "chunks", "created_at", "indexed_at" + } + + +def test_image_affordance_absent_without_the_map() -> None: + """No map (the default) → every file node is the pre-phase shape, + even for ``.png`` paths: the fields are map-driven (the endpoint + composes the map from the ``is_image`` rows), never path-guessed — + a pre-phase KB serializes byte-identically.""" + rows = [("S", "pic.png", "pic", 1, T0, C0)] + (source,) = build_kb_tree(["S"], rows, {}) + (file,) = _file_nodes(source) + assert set(file.model_dump()) == { + "kind", "path", "title", "chunks", "created_at", "indexed_at" + } diff --git a/tests/unit/test_save_as_doc_button.py b/tests/unit/test_save_as_doc_button.py index 7895a56..f2d87be 100644 --- a/tests/unit/test_save_as_doc_button.py +++ b/tests/unit/test_save_as_doc_button.py @@ -53,18 +53,23 @@ def test_app_config_dict_carries_the_docs_flag() -> None: body = app_config(s) # Phase 62 (task 01): the response grew to the phase-62 UI # customization keys; phase 91 (task 03) deleted the retired - # CSS-file theming's ``theme`` key — the five keys below are the - # entire endpoint contract. + # CSS-file theming's ``theme`` key; phase 122 (task 01) added the + # ``images`` flag — the six keys below are the entire endpoint + # contract. assert set(body) == { "app_name", "version", "docs_repo_configured", - "input_placeholder", "footer_text", + "images", "input_placeholder", "footer_text", } assert body["docs_repo_configured"] is s.docs_configured assert body["docs_repo_configured"] is False + assert body["images"] is False # LOCKED A3: off by default s2 = _settings(docs_repo="/srv/docs-repo") assert app_config(s2)["docs_repo_configured"] is True + s3 = _settings(images=True) + assert app_config(s3)["images"] is True + # --------------------------------------------------------------------------- # brand.js — the flag + promise are surfaced the way app_name is diff --git a/tests/unit/test_wide_column_css.py b/tests/unit/test_wide_column_css.py index a72f085..d441a00 100644 --- a/tests/unit/test_wide_column_css.py +++ b/tests/unit/test_wide_column_css.py @@ -114,23 +114,25 @@ def test_no_literal_46rem_width_remains() -> None: # ---------- the four reading-column selectors ---------- -def test_the_four_reading_columns_use_the_token() -> None: - """.chat-shell, .shared-shell, .doc-md and - .doc-summary:has(+ .doc-md) each cap with - max-width: var(--chat-column) — and exactly those four rules use - the token for a max-width (no other selector).""" +def test_the_reading_columns_use_the_token() -> None: + """.chat-shell, .shared-shell, .doc-md, .doc-image (phase 122, + task 04 — the viewer's image block rides the SAME reading column + as its .doc-md sibling) and .doc-summary:has(+ .doc-md) each cap + with max-width: var(--chat-column) — and exactly those five rules + use the token for a max-width (no other selector).""" css = _css() for selector in ( ".chat-shell", ".shared-shell", ".doc-md", + ".doc-image", ".doc-summary:has(+ .doc-md)", ): assert "max-width: var(--chat-column)" in _rule_block(css, selector), ( f"{selector} must cap with max-width: var(--chat-column)" ) - assert css.count("max-width: var(--chat-column)") == 4, ( - "exactly the four reading-column selectors use the token" + assert css.count("max-width: var(--chat-column)") == 5, ( + "exactly the reading-column selectors use the token" )