diff --git a/specs/0.7/01-inspector-rescue.md b/specs/0.7/01-inspector-rescue.md new file mode 100644 index 0000000..697a791 --- /dev/null +++ b/specs/0.7/01-inspector-rescue.md @@ -0,0 +1,270 @@ +# 0.7 · pdf-inspector as a Deterministic Rescue Candidate + +> Behavior spec. Delivery order lives in [`IMPLEMENTATION-PLAN.md`](IMPLEMENTATION-PLAN.md). + +## 1.1 The shape of the change + +Today `engine/remediate.py` builds a candidate list per page and adopts the best: + +``` +candidates = {vlm, cleanup} # both cost money, both call a model +winner = _pick_winner(...) # vlm > cleanup, else keep baseline +``` + +0.7 adds a third candidate that is free, deterministic, local, and ~5 ms/page: + +``` +candidates = {inspector, vlm, cleanup} +``` + +`pymupdf4llm` remains the baseline parser. `Source Document.parser_used` still reads +`pymupdf4llm`. Nothing in `engine/__init__.py::parse_pdf` changes. + +Two behavioral wins, in order of value: + +1. **Quality** — on pages the harness flags, inspector's text recovery is near-perfect + where the baseline collapses (`review`: 0.565 → 0.998; `escalate`: 0.505 → 0.977). +2. **Cost** — because it is computed *before* the paid candidates, a page it already + wins outright can skip the VLM and cleanup calls entirely (§1.5). + +## 1.2 The adapter — `wikify/engine/parsers/inspector.py` + +Mirrors the shape of `parsers/pymupdf.py`, but document-scoped rather than page-scoped, +because per-page calls are catastrophically slower (measured: 1170 ms/page single-call +vs 4.7–7.0 ms/page whole-document — `extract_pages_markdown` recomputes document-wide +font statistics on every invocation). + +```python +NAME = "inspector" + +def parse_document(pdf_path: str) -> dict[int, str]: + """1-based page number → markdown, for the whole document in one pass.""" +``` + +Implementation notes that are easy to get wrong: + +- `pdf_inspector.extract_pages_markdown(path)` returns a `PagesExtractionResult`, not a + list. It has **no `len()`**. Pages are at `.pages`, each a `PageMarkdown` with + `.page`, `.markdown`, `.needs_ocr`, `.ocr_reason`. +- **`PageMarkdown.page` is 0-based** (verified: `0..179` for a 180-page document). The + optional `pages=` argument is also 0-based. The adapter converts once, at the seam: + `{pg.page + 1: pg.markdown for pg in result.pages}`. Every caller above the adapter + works in Frappe's 1-based page space. +- The result object also exposes `is_complex`, `pages_needing_ocr`, `pages_with_tables`, + `pages_with_columns`. Not used in 0.7; noted so a future slice doesn't rediscover them. +- Wrap the whole call in `try/except` and return `{}` on failure. A missing wheel, an + encrypted PDF, or a panic in the Rust core must degrade to "no inspector candidate", + never abort a remediation run. + +Document-level classification (`detect_pdf` / `classify_pdf`) is **not** adopted in 0.7. +Both spike documents returned `text_based` at confidence 1.00 with zero pages needing +OCR, so the corpus does not exercise it and there is nothing to validate a replacement +of `pdf_utils.classify_page` against. Left for a later release with a scanned corpus. + +## 1.3 The structure guard — the load-bearing part of this spec + +Inspector's `content_recall` is 0.997. Its structural fidelity is **document-dependent +and fails silently.** Measured over 60 sampled pages per document: + +| | obg | neph | +|---|---|---| +| headings, baseline → inspector | 132 → **10** | 78 → 72 | +| pages with any `#` | 38/60 → **4/60** | 32/60 → 32/60 | +| list items | 320 → **0** | 326 → 283 | +| table rows | 35 → **220** | 359 → 198 | + +On OBG it deletes essentially every heading, emits **zero** list items, and converts +prose into wide pseudo-tables. `engine/sectionize.py` splits documents on `#` headings +(`loader/sectionizer.py:63-151`); adopting that output would flatten the section tree +while every score in the UI went *up*. `parser_artifacts` does not catch it — its +patterns are tuned to pymupdf4llm's signatures. + +So structure becomes an explicit, deterministic adoption gate. + +**New in `wikify/engine/verify/deterministic.py`** (pure stdlib, no frappe, consistent +with the rest of that module): + +```python +def structure_signature(markdown: str) -> dict: + """Counts of the markdown structure the downstream pipeline depends on: + {"headings", "list_items", "table_rows", "table_seps"}.""" + +def structure_preserved(base_md: str, cand_md: str) -> tuple[bool, str]: + """Whether `cand_md` keeps the document structure `base_md` established. + Returns (ok, reason) — reason is '' when ok, else a short cause for the notes.""" +``` + +`structure_preserved` rejects on any of three rules, each calibrated on the table above: + +| Rule | Condition | Rationale | +|---|---|---| +| `heading_loss` | base has ≥1 heading and candidate has `< 0.5 ×` base headings | sectionizer input; obg drops 132→10 | +| `list_wipe` | base has ≥3 list items and candidate has 0 | obg drops 320→0 across the sample | +| `table_inflation` | candidate table rows `> 3 ×` base table rows **and** base has ≥1 list item | prose-scrambled-into-table; obg 35→220 | + +Measured rejection behaviour with these exact thresholds, over the 60-page samples: + +| | heading_loss | list_wipe | table_inflation | +|---|---|---|---| +| obg | 36/60 | 27/60 | 3/60 | +| neph | 7/60 | 6/60 | 1/60 | + +That is the discrimination the guard exists to provide: it blocks the document where +inspector destroys structure and admits the one where it does not. The thresholds are +constants at the top of the module with the measurement in a comment, so a future +corpus can retune them against evidence rather than taste. + +**Additionally**, add a `prose_table` pattern to `engine/lint.py`'s `table_artifacts` +(consumed by `deterministic.parser_artifacts`): a table row with `> 6` cells whose mean +non-empty cell length exceeds ~40 characters is prose that has been scrambled into a +table. This closes the blind spot generally, not just for this parser, and rides the +existing composite penalty (`harness.py:107-110`, `composite *= 0.7`). + +## 1.4 Candidate construction and the winner rule + +In `engine/remediate.py::remediate_pdf`, before the page loop: + +```python +inspector_pages = inspector.parse_document(pdf_path) # {} on any failure +``` + +Inside the loop, for each target page, **before** the vlm/cleanup calls: + +```python +ins_md = inspector_pages.get(p["page_no"], "") +``` + +An inspector candidate is constructed only when **all** of: + +- `ins_md` is non-empty, and +- `kind != "visual"` — inspector has no image understanding, and scoring a visual page + requires a paid judge call (`VISUAL_WEIGHTS` is judge-dominant, `config.py:33`), which + would invert the cost saving this candidate exists to produce. + +It is **adopt-eligible** when both: + +- `ins_ps.composite > base_ps.composite` (same bar the vlm candidate clears), and +- `structure_preserved(base_md, ins_md)` returns ok. + +When the guard rejects, the candidate is still appended with `adopt_eligible=False` and +its rejection reason flows into `store.set_remediation`'s `notes`, so the review UI +shows *why* a high-scoring candidate was not taken. Silent rejection would be worse than +no guard. + +**`_pick_winner` changes minimally.** The existing vlm-over-cleanup rule and its +tie-to-vlm rationale (cleanup's composite is depressed by intended furniture removal) +are preserved exactly. Inspector is layered on top: + +``` +winner = existing vlm/cleanup resolution # unchanged +if inspector eligible and (no winner or inspector.composite >= winner.composite): + winner = inspector +``` + +Ties go to inspector: it is deterministic and free, so given equal scores it is the +better artifact to persist. Note this only affects *which* candidate is stored — by the +time both exist, both have already been paid for. The saving comes from §1.5. + +## 1.5 Short-circuit — where the money is saved + +Today every remediation target costs a VLM call, and non-visual pages cost a cleanup +call on top (`remediate.py:109-135`). On a 398-page manual with `scope="all"`, that is +~800 model calls. + +When the inspector candidate already clears the pass bar, the paid candidates are +pointless. Guarded by a new setting: + +``` +Wikify Settings.inspector_short_circuit (Check, default 1) +``` + +When enabled, and for a page where **all** of the following hold, the vlm and cleanup +calls are skipped entirely and the inspector candidate is adopted directly: + +- `kind != "visual"` +- inspector candidate exists and is adopt-eligible (score **and** structure guard) +- `ins_ps.composite >= settings.pass_threshold` (0.90 by default) + +The page's `remediation_method` records `inspector`; `notes` records +`short-circuit: skipped vlm/cleanup`. `llm.get_metrics()` for that page is empty, so +`Source Page.llm_cost` lands at 0 — which is the honest number and makes the saving +directly visible in the existing cost UI. + +Expected impact, from the spike: inspector reaches ≥0.90 composite on the large majority +of `review`-verdict pages (mean 0.998 there, n=42). Those pages currently consume two +model calls each and would consume none. The exact saving is corpus-dependent and is a +**measured acceptance criterion** of Slice 33, not a claim this spec makes in advance. + +Short-circuit is deliberately conservative: it never applies to visual pages, never +applies when the structure guard fires, and never applies below `pass_threshold`. A page +that inspector improves but does not fully rescue still gets the full paid treatment, +and the best of all three wins. + +## 1.6 Data model + +Additive only. No migration of existing rows. + +| DocType | Field | Change | +|---|---|---| +| `Source Page` | `remediation_method` | Select — add `inspector` to `'' / cleanup / vlm` | +| `Source Page` | `canonical_source` | Select — add `inspector` to `'' / baseline / cleanup / vlm / image` | +| `Wikify Settings` | `inspector_enabled` | **new** Check, default `1` — master switch for the candidate | +| `Wikify Settings` | `inspector_short_circuit` | **new** Check, default `1` — §1.5 gating | + +`Source Document.parser_used` is untouched and continues to read `pymupdf4llm`. The +baseline is the baseline. + +Frontend: wherever `canonical_source` / `remediation_method` are rendered as labels +(page review split-pane, remediation badges), add the `inspector` case. It should read +as a first-class method, not an unknown value falling through to a default. + +## 1.7 Dependency + +```toml +pdf-inspector>=0.2.6 # deterministic Rust PDF→markdown; rescue candidate only +``` + +- MIT, pure Rust core (`lopdf`), **no ML models, no torch, no ONNX**, fully offline. +- Ships `cp38-abi3` wheels for macOS arm64/x86_64, manylinux aarch64/x86_64, win_amd64 — + abi3 means the 3.8 wheel installs on the bench's Python 3.14. Verified installed and + working on this bench. +- Released 2026-07-31 at v0.2.6. **This is a young library.** That immaturity is the + reason it enters as a guarded candidate rather than the baseline, and the reason + §1.2's failure path degrades silently. + +While specifying this, note that `requests` is imported by `engine/llm.py:17` but +declared nowhere, and `frappe-wiki` is a hard runtime dependency of `engine/generate.py` +that is likewise undeclared. Out of scope for 0.7, worth a follow-up. + +## 1.8 Degradation + +| Condition | Behaviour | +|---|---| +| `pdf_inspector` not importable | candidate absent; remediation identical to 0.6 | +| `extract_pages_markdown` raises | `parse_document` returns `{}`; candidate absent | +| page missing from the map | candidate absent for that page only | +| `inspector_enabled = 0` | candidate never constructed | +| `inspector_short_circuit = 0` | candidate still competes, paid passes always run | +| structure guard fires | candidate ineligible, reason recorded in `notes` | + +In every row, the fallback is current behaviour. There is no state in which enabling +this feature can make a document fail to remediate. + +## 1.9 Tests + +Unit (`wikify/tests/`, hermetic, no network — LLM calls mocked as in +`test_remediate_pipeline.py`): + +- `test_structure_guard.py` — `structure_signature` counts on committed synthetic + markdown; each of the three `structure_preserved` rules fires on a crafted case and + stays quiet on a clean one; a control where candidate structure is *richer* passes. +- `test_inspector_adapter.py` — 1-based conversion of a stubbed `PagesExtractionResult`; + `{}` on raised exception; `{}` when the module is absent. +- `test_remediate_pipeline.py` (extend) — inspector candidate adopted when it wins and + preserves structure; **rejected despite a higher composite** when the guard fires + (the regression that matters most); short-circuit skips vlm/cleanup and records zero + cost; short-circuit never fires on `kind="visual"`. +- `test_lint.py` (extend) — `prose_table` fires on a wide long-celled table, stays quiet + on a normal one. + +Live acceptance is per-slice in the plan, against `pdf.localhost`. diff --git a/specs/0.7/02-semantic-retrieval.md b/specs/0.7/02-semantic-retrieval.md new file mode 100644 index 0000000..e47f44a --- /dev/null +++ b/specs/0.7/02-semantic-retrieval.md @@ -0,0 +1,256 @@ +# 0.7 · Semantic Retrieval over Generated Wiki Pages + +> Behavior spec. Delivery order lives in [`IMPLEMENTATION-PLAN.md`](IMPLEMENTATION-PLAN.md). + +## 2.1 The gap, measured + +`agent/tools/read.py:132-173` is the only cross-document search in the product. It +filters `Source Section` by `section_type`, then does a Python `in` substring test +against `title` and `hierarchy_path`. **It never reads a body.** + +Measured against the real OBG wiki (358 documents, 180 generated questions): + +| Arm | recall@1 | recall@5 | recall@10 | MRR@10 | +|---|---|---|---|---| +| substring (today) | 0.000 | 0.000 | **0.000** | 0.000 | +| full-text (tantivy) | — | — | 0.611 | 0.417 | +| **vector (bge-small)** | — | — | **0.794** | **0.534** | +| hybrid, default RRF | — | — | 0.728 | 0.528 | + +Zero on all 180. That is not a tuning problem: the tool asks whether the *entire query +string* is a substring of a title, and a real question never is. + +By query type — the reason this release picks vector over full-text: + +| Query type | FTS | Vector | Δ | +|---|---|---|---| +| keyword | 0.883 | 0.917 | +0.03 | +| **semantic** (paraphrased) | 0.433 | **0.750** | **+0.32** | +| **vague** | 0.517 | **0.717** | **+0.20** | + +Keyword retrieval is already a solved problem here — `frappe-wiki` ships +`WikiSQLiteSearch` (FTS5 over `Wiki Document.title` + `content`, registered via the +`sqlite_search` hook in `wiki/hooks.py:29`, live at `sites//wiki_search.db`). 0.7 +does not duplicate it. It adds the axis FTS structurally cannot serve. + +**Caveats carried forward honestly.** The gold set is synthetic — an LLM wrote each +question while looking at its target page, which inflates absolute recall for every arm; +the *gaps between arms* are the trustworthy signal. The embedding model in the spike was +local `bge-small-en-v1.5` because no embedding key exists on the bench, so 0.794 is a +**lower bound** for the hosted model this spec adopts. + +## 2.2 Scope + +Index target is **`Wiki Document`** — the generated wiki pages, chosen by the product +owner. Not `Source Section`, and not the hard-deprecated `Wiki Page`. + +Consequence to accept knowingly: content is only searchable *after* wiki generation. An +import sitting in `Review` has nothing in the index. `Source Section` carries richer +filter metadata (`page_start`/`page_end`, `section_type`) that this choice forgoes; the +back-link is preserved (`Source Section.wiki_document`) so a later slice can join or +switch targets without a migration. + +## 2.3 The seam — `wikify/engine/retrieval/` + +New package, deliberately narrow, mirroring how `engine/store.py` isolates the ORM: + +| Module | Responsibility | +|---|---| +| `retrieval/embed.py` | text → vectors via litellm. Batching, truncation, model/key resolution. | +| `retrieval/index.py` | LanceDB connection, schema, upsert, delete, search, optimize. | +| `retrieval/__init__.py` | The only public surface: `index_wiki_space`, `remove_document`, `search`, `is_configured`. | + +Nothing outside this package imports `lancedb` or `litellm.embedding`. + +### Storage + +Embedded LanceDB, one directory per site: + +``` +sites//private/files/lance/ +``` + +Resolved with `frappe.get_site_path("private", "files", "lance")` — per-site isolation +for free, inside the private files tree so it is never web-served, and it rides existing +backup/permission conventions. No server process, no new port, no daemon. + +Table `wiki_chunks`, one row per chunk: + +| Column | Type | Notes | +|---|---|---| +| `id` | str | `f"{wiki_document}::{chunk_ix}"` — stable, so re-index is an upsert | +| `wiki_document` | str | the Frappe docname; the retrieval unit | +| `source_document` | str | for project/document scoping | +| `project` | str | filter axis for the agent tool | +| `title` | str | returned for display | +| `route` | str | returned so the agent can link | +| `chunk_ix` | int | ordinal within the page | +| `text` | str | the chunk body | +| `vector` | vector(N) | N from the configured model | + +### Chunking + +1800 characters, 200 overlap — the spike's configuration, which produced 396 chunks from +358 documents (most wiki pages fit in one chunk; median content length is 368 chars). +Split on paragraph boundaries where possible, hard-split only when a paragraph exceeds +the window. + +A query returns **documents, not chunks**: score each `wiki_document` by the max score +over its chunks, deduplicate, then rank. Retrieving the same page three times because it +chunked three ways is a bug, not a result. + +Skip documents with `is_group = 1` and trivial bodies (< 400 characters) — these are ToC +and grouping nodes whose content is a generated index. The spike skipped 25 of 383 nodes +this way. Log the skip count; never skip silently. + +### Embeddings + +`litellm.embedding` (v1.83.7 on the bench, confirmed present — `litellm` is already a +declared dependency, so this adds **no new Python package**). + +| Setting | Default | +|---|---| +| `Wikify Settings.embedding_model` | `openai/text-embedding-3-small` | +| `Wikify Settings.embedding_api_key` | Password, empty | + +Key resolution follows the existing `engine/settings.py:23-82` ladder exactly (Settings +password → `site_config` → env → `apps/wikify/.env`) so operators configure this the way +they already configure `OPENROUTER_KEY`. **OpenRouter has no embeddings endpoint** — this +is a separate key and the settings UI must say so plainly. + +Batch inputs (the LanceDB agent guidance is explicit that row-at-a-time ingestion is the +wrong shape). Cost is negligible: ~400 chunks per manual against +`text-embedding-3-small` is a fraction of a cent. + +Vector dimension is read from the first embedding response rather than hardcoded, so +swapping models is a reindex and not a code change. A dimension change against an +existing table is a rebuild, not a migration — see §2.7. + +### Fork discipline — non-negotiable + +Frappe's RQ workers fork per job, and LanceDB's Rust core is multithreaded internally. +Forking a process that has already initialised Lance's thread pool is the documented way +to deadlock it. + +Therefore: **the connection is opened lazily inside the job body, never at module +import.** `index.py` holds a module-level `_db = None` and a `_connect()` that populates +it on first use. No `lancedb.connect()` at import time, no connection cached on a +long-lived object that predates the fork. A spike subprocess opening a fresh connection +and querying worked cleanly (0.96 s) — one observation, not a stress test, which is +exactly why the discipline is a rule rather than a hope. + +Writes are single-writer by construction: only the generate/reindex job writes. Readers +(web workers serving the agent tool) are concurrent and safe under Lance's MVCC. Call +`table.optimize()` after an ingest run to compact fragments. + +## 2.4 Keeping the index in sync + +`Wiki Document.content` is currently written from **five** places in +`engine/generate.py`: + +- `_upsert_wiki_document:35` (assigns `doc.content` at `:59`) +- the two-pass link rewrite — direct `frappe.db.set_value` at `:241` and `:261` +- `sync_section:352` — the 0.3 per-section push +- the deletion sweep at `:170` (`frappe.delete_doc`) + +Five writers is four too many to hook individually. 0.7 consolidates them behind one +funnel, the same move 0.6 made for `Source Section.markdown` +(`store.set_section_markdown`, `store.py:280-290`) and 0.5 made for reference extraction: + +```python +# wikify/engine/store.py +def set_wiki_content(name: str, content: str, *, update_modified: bool = False) -> None: + """THE write funnel for `Wiki Document.content` (0.7) — every content write goes + through here so the retrieval index always reflects the stored body.""" +``` + +**Indexing does not ride the funnel synchronously.** Embedding is a network call; a +generation run that writes 358 pages must not make 358 of them inline. Instead: + +- the funnel marks the document dirty (in-run set, or `Wiki Document.modified`), +- `jobs/generate.py::run` calls `retrieval.index_wiki_space(...)` **once** at the end, + after generation completes, batching every changed page, +- `generate.sync_section` (the single-page path, already interactive and already slow) + indexes that one document inline, +- the deletion sweep calls `retrieval.remove_document(name)`. + +A new `wikify.api.retrieval.reindex` whitelisted method enqueues a full rebuild for a +space, for operator recovery and after an `embedding_model` change. + +Index build cost, measured: 57.6 s for 358 documents including model load, extrapolating +to ~100 s for a 398-page manual. On-disk 1.35 MB, extrapolating to ~3 MB. Query latency +7.3 ms. None of this needs a progress bar. + +## 2.5 The agent tool + +New tool in `agent/tools/read.py`, registered alongside the existing read tools: + +``` +search_wiki(query: str, project: str | None = None, limit: int = 5) +``` + +Returns ranked `{wiki_document, title, route, score, excerpt}`. Results are capped at the +module's existing `_BODY_LIMIT` (6000 chars, `read.py:17`) — the same budget discipline +every other read tool follows. + +`search_sections` is **kept, not replaced**. It answers a genuinely different question +("show me every section of type X"), which is a metadata filter and correct as designed +(`api/explore.py:3-7`). Its docstring gains a line pointing at `search_wiki` for content +questions, so the model stops reaching for the wrong instrument — the 0.000 measurement +is substantially a tool-selection failure as well as a capability gap. + +Pure vector search. **Not hybrid.** The spike measured default equal-weight RRF hybrid +losing to plain vector (0.728 vs 0.794): hybrid wins on keyword queries but the weak FTS +ranking drags fusion down on exactly the semantic and vague queries the agent actually +issues. Hybrid returns when there is a tuned weight and a measurement to justify it. + +## 2.6 Degradation + +| Condition | Behaviour | +|---|---| +| no `embedding_api_key` | `is_configured()` false → indexing skipped with one log line; `search_wiki` returns "semantic search is not configured" | +| `lancedb` not importable | same as above; nothing else in the app imports it | +| embedding call fails mid-index | that batch is logged and skipped; generation still completes | +| index directory missing/corrupt | `search_wiki` reports the index is unbuilt and names the reindex action | +| space never generated | empty result, not an error | + +**Wiki generation must never fail because retrieval failed.** Indexing is a post-step +wrapped so its exceptions are logged, not raised. The index is derived and disposable +(README principle 4) — losing it costs one reindex. + +## 2.7 Reindex, not migrate + +The index is a projection. There is no schema migration path and there should not be: +changing `embedding_model`, changing the chunk window, or a corrupt directory are all +resolved by dropping the table and rebuilding from Frappe. + +`api.retrieval.reindex(wiki_space)` enqueues exactly that. Changing `embedding_model` in +`Wikify Settings` must warn that existing vectors become invalid until a reindex runs — +a table containing two models' vectors returns silently garbage rankings, which is the +worst failure mode available here. + +## 2.8 Evaluation + +Retrieval quality is a number, so 0.7 keeps measuring it. Port the spike harness into +`wikify/tests/evals/retrieval/`, alongside the existing live agent evals (which are +deliberately excluded from `run-tests` because they cost real tokens — +`tests/evals/__init__.py:1-17`): + +- `build_gold_set.py` — samples N wiki documents, generates keyword/semantic/vague + questions via OpenRouter, **caches to a committed JSON** so reruns are free and + comparable across changes. The cached gold set is the artifact; regenerating it resets + the baseline. +- `run_retrieval_eval.py` — recall@{1,3,5,10} and MRR@10 per arm and per query type. +- Run manually, matching the existing convention: + +```bash +bench --site pdf.localhost execute wikify.tests.evals.retrieval.run +``` + +The existing eval harness is boolean pass/fail (`scenarios.py:26`); this one is numeric +and reports a table. It is a tracked metric, not a gate — the acceptance bar for Slice 34 +is stated in the plan. + +Unit tests (hermetic, embeddings mocked): chunking boundaries and overlap; max-score +document dedup across chunks; `is_configured()` false paths; `set_wiki_content` funnel +coverage; `remove_document` on the delete sweep. diff --git a/specs/0.7/IMPLEMENTATION-PLAN.md b/specs/0.7/IMPLEMENTATION-PLAN.md new file mode 100644 index 0000000..8663b07 --- /dev/null +++ b/specs/0.7/IMPLEMENTATION-PLAN.md @@ -0,0 +1,222 @@ +# 0.7 Implementation Plan — Tracer-Bullet Slices + +Continues the spine (0.2: 10–16 · 0.3: 17–20 · 0.4: 21–25 · 0.5: 26–28 · 0.6: 29–31). +Numbering starts at **32**. Each slice cuts through every layer it touches and ends +demoable on its own. + +> Source of truth for behavior is [`01-inspector-rescue.md`](01-inspector-rescue.md) and +> [`02-semantic-retrieval.md`](02-semantic-retrieval.md). This file is the *delivery +> order*. + +## Slice map + +| # | Slice | Type | Blocked by | Status | +|---|---|---|---|---| +| 32 | Structure guard + inspector adapter — `structure_signature`/`structure_preserved`, `parsers/inspector.py`, `prose_table` lint pattern | AFK | — | ⬜ | +| 33 | Inspector in remediation — candidate + winner rule, short-circuit, settings, DocType Selects, UI labels | AFK | 32 | ⬜ | +| 34 | Retrieval spine — `engine/retrieval/`, `set_wiki_content` funnel, index on generate, reindex API, eval harness | AFK | — | ⬜ | +| 35 | `search_wiki` agent tool — registration, project scoping, `search_sections` docstring, settings UI | HITL | 34 | ⬜ | + +**Spine:** two independent tracks. 32 → 33 strictly sequential (33 adopts what 32 can +prove safe — building 33 first would ship the exact silent-flattening failure the spike +found). 34 → 35 strictly sequential (35 queries what 34 populates). The tracks share no +files and can land in either order. + +The guiding move: **32 is pure backend and independently valuable** — the structure guard +and the `prose_table` pattern improve artifact detection for *every* parser, inspector or +not, and can ship before any decision about adopting inspector output. **34 is likewise +independently valuable**: the index and funnel are useful the moment they exist, even +before a tool queries them. + +Land 32 first. It is the smallest slice, it de-risks 33 entirely, and its guard is the +one piece of this release that protects against a regression the current scoring cannot +see. + +--- + +## Verification + +Same protocol as 0.2–0.6: verify each slice against **`pdf.localhost`** (Administrator / +admin) before starting the next; `bench` from the bench root; `bench start` running for +anything touching jobs/realtime; `run-tests --app wikify` stays green throughout +(currently 180 tests across 19 modules). + +Standing acceptance fixtures — the two real manuals are the test bed: + +- `0pvfokvg4h` "Obstetrics and Gynaecology" (180 pp, `Wiki-Generated`, space + `0rkkb8s039`, 358 indexable wiki documents) — **the structure-hostile document.** + pdf-inspector drops headings 132→10 and list items 320→0 on the sampled pages here. + Any inspector work must be verified against this document specifically. +- `1svt8pm07l` "Nephrology" (398 pp, `Parsed`) — **the structure-friendly document** + (headings 78→72, lists 326→283). Inspector should be *adopted* on a meaningful number + of pages here. If it is adopted on neither document, the guard is too tight; if it is + adopted widely on obg, the guard is broken. +- Unit fixtures: committed synthetic markdown per guard rule + clean controls (the + fixture-leak rule from f79eb48 applies — no site data in unit tests). + +Reference numbers from the 2026-08-05 spike, for comparison during verification: + +| | pymupdf4llm | pdf-inspector | +|---|---|---| +| `content_recall`, `review` pages (n=42) | 0.565 | 0.998 | +| `content_recall`, `escalate` pages (n=4) | 0.505 | 0.977 | +| whole-document extract | 258–272 ms/pg | 4.7–7.0 ms/pg | + +--- + +## 32 — Structure guard + inspector adapter + +**Demo:** in `bench console`, feed the guard a real pair of markdown strings from the OBG +manual (baseline vs inspector) → it returns `(False, "heading_loss")`. Feed it the +Nephrology equivalent → `(True, "")`. `parsers/inspector.py` returns a 1-based +`{page_no: markdown}` map for a 398-page PDF in under two seconds. + +### What to build + +- `engine/verify/deterministic.py`: `structure_signature`, `structure_preserved`, and the + three threshold constants with the measurement recorded in a comment (§1.3). +- `engine/parsers/inspector.py`: `NAME`, `parse_document(pdf_path) -> dict[int, str]`. + **`PageMarkdown.page` is 0-based** — convert at this seam and nowhere else. Return `{}` + on any exception or missing module. +- `engine/lint.py`: `prose_table` pattern in `table_artifacts` (> 6 cells, mean non-empty + cell length > ~40 chars). +- `pyproject.toml`: `pdf-inspector>=0.2.6`. +- Tests per §1.9 (`test_structure_guard.py`, `test_inspector_adapter.py`, `test_lint.py` + extension). + +### Acceptance criteria + +- `parse_document` on Nephrology (398 pp) completes in < 3 s and returns 398 entries + keyed 1–398, with entry `1` matching page 1 of the PDF (off-by-one is the failure mode + this criterion exists to catch). +- Over the OBG sample, `structure_preserved` rejects on the order of half the pages; + over Nephrology, it rejects roughly one in eight. Exact counts will move with sampling — + what must hold is the **direction**: obg rejection rate is several times Nephrology's. +- Uninstalling `pdf_inspector` leaves `run-tests` green. +- `prose_table` does not fire on any existing `Source Section` that is currently lint-clean + (check against the live site before committing the threshold). + +--- + +## 33 — Inspector in remediation + +**Demo:** run remediation over Nephrology with `bench start` up → the log stream shows +pages adopted with method `inspector` at zero cost, and the import's total `llm_cost` is +materially below a pre-0.7 run of the same document. Run it over OBG → inspector is +attempted and mostly rejected, with `heading_loss` visible in the page's remediation +notes, and the section tree is unchanged. + +### What to build + +- `engine/remediate.py`: document-level `inspector.parse_document` before the page loop; + candidate construction (non-visual, non-empty, guard-gated); `_pick_winner` extension + preserving the existing vlm/cleanup rule; short-circuit per §1.5. +- `Wikify Settings`: `inspector_enabled`, `inspector_short_circuit` (both Check, default + 1) + the settings UI fields. +- `Source Page`: add `inspector` to the `remediation_method` and `canonical_source` + Selects. `bench --site pdf.localhost migrate`. +- Frontend: `inspector` label case wherever `canonical_source` / `remediation_method` are + rendered. +- Tests per §1.9 — including the **guard-rejects-higher-composite** case, which is the + regression that protects the section tree. + +### Acceptance criteria + +- On Nephrology, `inspector` is adopted on a non-trivial share of remediation targets and + `canonical_mean` does not regress versus a pre-0.7 run. +- On OBG, section tree shape (`_tree_shape`-style comparison: names + parentage) is + **identical** before and after enabling the feature. This is the criterion that proves + the guard works end-to-end. +- Measured LLM-call saving on a full `scope="all"` Nephrology run is recorded in the + slice's verification note — an actual number, not an estimate. This is the slice's + headline result. +- `inspector_short_circuit = 0` reproduces 0.6 cost behaviour exactly. +- Short-circuit never fires on a `visual` page (assert, don't assume). +- Every rejected-but-scoring candidate leaves its reason in `Source Page.remediation_notes`. + +--- + +## 34 — Retrieval spine + +**Demo:** `bench --site pdf.localhost execute wikify.api.retrieval.reindex --kwargs "{'wiki_space': '0rkkb8s039'}"` +→ ~358 documents embedded and indexed in ~100 s → a console query for *"heavy bleeding +after delivery"* returns the postpartum haemorrhage page in the top 3, which the current +substring search cannot do at any k. + +### What to build + +- `engine/retrieval/{__init__,embed,index}.py` per §2.3 — lazy `_connect()`, **no + module-import-time connection** (fork discipline). +- `Wikify Settings`: `embedding_model` (default `openai/text-embedding-3-small`), + `embedding_api_key` (Password), with help text stating this is *not* the OpenRouter key. +- `store.set_wiki_content` funnel; route all five `engine/generate.py` write sites through + it (`_upsert_wiki_document:35/:59`, `:241`, `:261`, `sync_section:352`) and wire the + delete sweep at `:170` to `remove_document`. +- `jobs/generate.py::run`: batched `index_wiki_space` as a post-step, exception-wrapped. +- `api/retrieval.py`: whitelisted `reindex(wiki_space)` enqueuing a rebuild. +- `wikify/tests/evals/retrieval/` per §2.8, with the gold set committed. +- Unit tests: chunking, max-score dedup, `is_configured()` false paths, funnel coverage. + +### Acceptance criteria + +- With no `embedding_api_key`, wiki generation completes normally and logs one skip line. + **Verify this before verifying the happy path** — degradation is the risk, indexing is + the easy part. +- Reindex of `0rkkb8s039` completes; on-disk size is single-digit MB; `table.optimize()` + runs. +- `run_retrieval_eval` reports recall@10 ≥ 0.70 against the committed gold set. The spike + measured 0.794 with a *local* model; a hosted model under-performing that bar means + something is wrong with chunking or dedup, not with the premise. +- Substring-baseline recall is re-measured and reported alongside, to keep the comparison + honest as the corpus changes. +- Regenerating a wiki twice does not duplicate rows (`id` upsert is stable). +- Deleting a section that owns a wiki document removes its rows from the index. + +--- + +## 35 — `search_wiki` agent tool + +**Demo:** in the chat panel on a generated wiki, ask *"where does this manual cover +managing a patient with heavy post-delivery bleeding?"* → the agent calls `search_wiki`, +gets the right page, and answers with a link — with no attachment chip and no mention of +the page's title in the question. + +### What to build + +- `agent/tools/read.py`: `search_wiki(query, project=None, limit=5)` registered with the + other read tools; results capped at `_BODY_LIMIT`; excerpt + `route` returned. +- `search_sections` docstring line pointing at `search_wiki` for content questions. +- Unconfigured path returns a clear, actionable message the model can relay. +- Extend `wikify/tests/evals/` with a scenario asserting `"search_wiki" in + turn["tools_used"]` for a content question — tool *selection* is half the fix, since the + 0.000 baseline is partly the model reaching for the wrong instrument. + +### Acceptance criteria + +- A content question with no attachment chip is answered from the right page. +- A type-filter question (*"show me all the Procedure sections"*) still routes to + `search_sections`, not `search_wiki`. +- With retrieval unconfigured, the agent says so plainly instead of hallucinating or + silently returning nothing. +- Tool results respect `_BODY_LIMIT`; a broad query cannot blow the context budget. + +--- + +## Out of scope, recorded so it is not mistaken for fallout + +- **`engine/images.py:71-103` Case 1 is already dead code on this corpus.** A full + 578-page scan of both manuals found pymupdf4llm's + `==> picture [W x H] intentionally omitted <==` marker fires **0 times**. Pre-existing, + unrelated to 0.7, worth its own slice. +- **`pymupdf4llm` is silently invoking Tesseract OCR** on hundreds of pages in both + manuals (visible on stderr during any parse), which is where its ~270 ms/page goes. Not + addressed here; a likely source of further speed-up. +- **Undeclared dependencies:** `requests` (`engine/llm.py:17`) and `frappe-wiki` + (`engine/generate.py`, `api/wiki.py`) are imported but absent from `pyproject.toml`. +- **`remediation_workers` / `classify_workers`** settings are stored and shown in the UI + but never read — both passes are deliberately sequential + (`engine/remediate.py:18-20`, `engine/classify.py:6-12`). +- **Hybrid search and reranking** — deferred until there is a tuned weight and a + measurement showing it beats pure vector (§2.5). +- **Document-level PDF classification** via `detect_pdf` — deferred until there is a + scanned corpus to validate it against (§1.2). diff --git a/specs/0.7/README.md b/specs/0.7/README.md new file mode 100644 index 0000000..ab334b7 --- /dev/null +++ b/specs/0.7/README.md @@ -0,0 +1,136 @@ +# Wikify 0.7 — Deterministic Rescue & Semantic Retrieval + +Two measured gaps, one release. Both were validated by offline spikes against the live +corpus on **2026-08-05** before a line of this spec was written; every threshold below +comes from those runs, not from intuition. + +**Gap 1 — remediation pays an LLM to fix what a free parser already fixes.** +`jobs/parse.py:74-76` sends *every* page through the remediation pass, so a 398-page +manual costs ~800 model calls. Yet on the pages the harness flags `review`/`escalate`, +a purely deterministic Rust parser (`pdf-inspector`) recovers the text almost perfectly +— mean `content_recall` **0.565 → 0.998** on `review` pages, **0.505 → 0.977** on +`escalate` — at ~5 ms/page and zero cost. The expensive VLM pass is being spent on +pages a free candidate could win outright. + +**Gap 2 — nothing in the product can find content by meaning.** +`agent/tools/read.py:158-163` substring-matches the query against `title` and +`hierarchy_path` and never reads a body. Measured over 180 generated questions against +the real OBG wiki, it scored **recall@10 = 0.000 on all 180**. Not "weak" — zero. A +chat agent over a wiki cannot currently locate a page by what it says. + +**0.7 closes both**: `pdf-inspector` joins remediation as a free, deterministic +*candidate* (never the baseline) behind a structure guard, and short-circuits the LLM +passes when it already wins; and a LanceDB vector index over generated wiki pages gives +the agent real semantic retrieval. + +## Spec index + +| Doc | Covers | +|---|---| +| [`01-inspector-rescue.md`](01-inspector-rescue.md) | The `pdf-inspector` adapter, document-level batch extraction, the structure-preservation guard, candidate eligibility + winner rules, LLM short-circuit, artifact patterns, settings, degradation. | +| [`02-semantic-retrieval.md`](02-semantic-retrieval.md) | The LanceDB seam, wiki-page chunking + embedding via litellm, the `set_wiki_content` write funnel, reindex job, the agent search tool, fork discipline, degradation, and the retrieval eval harness. | +| [`IMPLEMENTATION-PLAN.md`](IMPLEMENTATION-PLAN.md) | Tracer-bullet slices **32–35** (continuing 0.6's numbering), delivery order, per-slice Verify steps against `pdf.localhost`. | + +## The evidence (spike, 2026-08-05) + +Both spikes ran offline against `files/Obstetrics and Gynaecology.pdf` (180 pp) and +`files/Nephrology.pdf` (398 pp), scored with the project's own +`engine/verify/deterministic.py`. Raw data and reports are **not** checked in; the +numbers that matter are reproduced here and in the two specs. + +**Parser A/B** — 120 pages, 60 per document, stratified by `lab.db` verdict, 0 failures +either arm: + +| Metric (pooled, n=120) | pymupdf4llm | pdf-inspector | +|---|---|---| +| `content_recall` mean | 0.666 | **0.997** | +| `extra_ratio` mean | 0.318 | **0.011** | +| pages with `parser_artifacts` | 34% | 0% | +| whole-document extract | 258–272 ms/pg | **4.7–7.0 ms/pg** | + +**But structure is document-dependent and fails silently.** Same 120 pages, counting +markdown structure markers: + +| | obg: pymupdf4llm → pdf-inspector | neph: pymupdf4llm → pdf-inspector | +|---|---|---| +| headings | 132 → **10** | 78 → 72 | +| pages with any `#` | 38/60 → **4/60** | 32/60 → 32/60 | +| list items | 320 → **0** | 326 → 283 | +| table rows | 35 → **220** | 359 → 198 | + +On the OBG manual `pdf-inspector` emits almost no headings, **zero** list items, and +scrambles prose into wide pseudo-tables. `parser_artifacts` scores that 0% because its +patterns are tuned to pymupdf4llm's failure signatures and are blind to this one. That +is the entire reason for §1.3's structure guard, and the reason `pdf-inspector` must +never become the baseline parser. + +**Retrieval** — 358 wiki documents from space `0rkkb8s039`, 180 LLM-generated questions +(60 docs × keyword/semantic/vague), local `bge-small-en-v1.5` embeddings: + +| Arm | recall@10 | MRR@10 | +|---|---|---| +| substring (today) | **0.000** | 0.000 | +| full-text (tantivy) | 0.611 | 0.417 | +| **vector** | **0.794** | **0.534** | +| hybrid, default RRF | 0.728 | 0.528 | + +By query type, vector's margin over FTS is +0.03 on keyword, **+0.32 on semantic**, +**+0.20 on vague** — the gap appears exactly where a chat agent lives. + +## Principles locked (2026-08-05) + +1. **`pdf-inspector` is a candidate, never the baseline.** `pymupdf4llm` stays the + baseline parser and the sole owner of `Source Document.parser_used`. Inspector + output is only ever adopted per-page, through the existing best-of-N adoption in + `engine/remediate.py`, and only when it wins on score *and* passes the structure + guard. The section tree is built from headings; a parser that scores 0.997 on recall + while deleting every heading would silently flatten the product. +2. **Free candidates run first and can pre-empt paid ones.** Inspector costs nothing + and takes ~5 ms/page. It is computed before the VLM/cleanup calls, and when it + already clears `pass_threshold` the paid passes are skipped entirely. This is the + cost win; adoption quality is unchanged either way. +3. **Structure is a first-class adoption criterion.** Recall answers "did we keep the + words". It cannot answer "did we keep the document". 0.7 adds a deterministic + structure signature to `verify/deterministic.py` and makes it a hard gate — measured + to reject 36/60 bad OBG adoptions while passing 53/60 on Nephrology. +4. **Retrieval is derived and disposable.** The LanceDB index is a projection of + `Wiki Document.content`, fully rebuildable from Frappe at any time. It is never a + source of truth, it is never migrated, and losing the directory costs one reindex. + Same stance `Section Reference` takes toward section markdown. +5. **Absent configuration degrades, never breaks.** No embedding key → indexing is + skipped and the search tool reports itself unconfigured; wiki generation is + unaffected. `pdf-inspector` failing to import or throwing → the candidate is simply + absent and remediation proceeds exactly as it does today. +6. **Ship pure vector, not hybrid.** The spike measured default equal-weight RRF hybrid + *losing* to plain vector (0.728 vs 0.794) because the weak FTS ranking drags the + fusion down. Hybrid is deferred until there is a measured reason and a tuned weight. + +## Decisions (confirmed 2026-08-05) + +- **Placement:** new `specs/0.7/`, slices **32–35**, continuing 0.6 (29–31). No + dependency on 0.5's graph surface. +- **Inspector scope:** runs on remediation targets only — never in `parse_pdf`'s page + loop, never on `visual` pages (it has no image understanding and would force a paid + judge call to score). +- **Batch, don't page.** `extract_pages_markdown` recomputes document-wide font stats + per call: 1170 ms for a single page versus 4.7–7.0 ms/page extracted whole-document. + Extraction happens once per `remediate_pdf` run, before the page loop. +- **Index target:** `Wiki Document` (generated wiki pages), not `Source Section` — + chosen by the product owner. `Wiki Page` is hard-deprecated and out of scope. +- **Embeddings:** hosted via `litellm.embedding` (already a dependency, v1.83.7 on the + bench). Default `openai/text-embedding-3-small`. OpenRouter has no embeddings + endpoint, so this needs its own key in `Wikify Settings`. +- **Storage:** LanceDB embedded, one directory per site under + `sites//private/files/lance/`. No server process, no new port. +- **`images.py` is out of scope but flagged.** A full 578-page scan found pymupdf4llm's + `==> picture [W x H] intentionally omitted <==` marker fires **0 times** on either + document, so `engine/images.py:71-103` Case 1 is already dead code on this corpus. + That is a pre-existing defect, unrelated to this release; it is recorded here so it + is not mistaken for 0.7 fallout. + +## Conventions (unchanged) + +Same as [`../0.6/README.md`](../0.6/README.md): backend per the `frappe-app-dev` skill, +engine work behind the `store.py` seam, thin whitelisted APIs, frontend frappe-ui v1 + +semantic tokens, verify every slice against `pdf.localhost` before the next, work +directly on `main`.