feat(rag): LanceDB retrieval, cited answers, and PDF fidelity fixes - #3
Open
Rl0007 wants to merge 8 commits into
Open
feat(rag): LanceDB retrieval, cited answers, and PDF fidelity fixes#3Rl0007 wants to merge 8 commits into
Rl0007 wants to merge 8 commits into
Conversation
`bench build --app wikify` silently skipped the frontend: frappe's esbuild runner looks for `apps/<app>/package.json` and `continue`s when it is absent (frappe/esbuild/esbuild.js:611-620), so no bundle and no `www/wikify.html` were ever emitted on a fresh `bench get-app`. Add the root package.json with the standard Frappe SPA scripts, matching frappe/crm. `postinstall` also keeps frontend deps in sync — without it a stale `frontend/node_modules` silently builds against the wrong frappe-ui. Also ignore `wikify/public/node_modules`: `bench build` symlinks it, and git sees a symlink rather than a directory, so the existing `node_modules/` rule never matched it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Too many files changed for review (130 files, 100 file limit). Bypass the limit by tagging |
Adds a retrieval layer and an Ask/RAG Lab UI over parsed documents, and fixes several pipeline defects found while running two real ICAI study PDFs through it. Retrieval (wikify/rag/) - LanceDB store with local model2vec embeddings (256d, no API key, no server). - Four modes: vector, full-text, hybrid (RRF), and filter. Filter returns EVERY match — "give me all X" is a metadata question, not a similarity guess. - Contextual-retrieval prefix on embed text, parent-section expansion, optional LLM rerank that degrades to fusion order when unavailable. - Permissions are a LanceDB pre-filter, never a post-hoc trim. An omitted ACL decision throws rather than searching everything. Answering - Intent router (exhaustive/semantic/hybrid) with follow-up rewriting; the decision and its plain-language reason are shown in the UI. - Answers cite [n] markers; each citation carries a resolved page, line span and verbatim quote, verified against the source before display. Verification is two gates: fuzzy prose match AND exact equality on figures, signs and statutory refs — a flipped (+)/(-) previously passed a 0.85 similarity check. - Honest refusal when nothing retrieved clears the bar. - Per-question cost and token usage returned and displayed. - Conversations persist to Wikify Ask Session/Message, so route decisions and citations become an evaluation set. Pipeline fixes - Sectioning stopped at the first heading longer than the 140-char title column, after the old tree was deleted — silently dropping 93.6% of a 236-page document. Titles are now clipped; coverage went 6.4% -> 99.8%. - Page classification gated on `chars < 250 AND drawings > 40`, which no born-digital diagram page can satisfy; replaced with per-region detection (1/236 -> 231/236 pages routed correctly). - Tables were being encoded as mermaid flowcharts, destroying row-to-rate correspondence; they now emit HTML tables, and diagrams are parsed and repaired (quoting node labels, `&` chains) before storage. - Page verdicts were frozen at the baseline parse, so remediated pages still read "review"; they now track the adopted content, with a backfill patch. - Remediation could adopt a near-empty candidate over a good one. - Page edits now propagate into the sections and index built from them. Frontend - /ask and /rag-lab, with sources rendered before the answer, citation chips, provenance, and a naive-vs-routed comparison that shows what similarity search missed. Unranked results no longer render a meaningless full score bar. - Mobile support across the app: shell navigation, drill-down replacements for desktop splits, and graph views that degrade to a list rather than an unreadable canvas. Evaluation - 12 golden questions with recall, precision and completeness, plus an HTML scorecard. Routed retrieval beats naive 66% -> 87-90% recall on the demo corpus; routing is an LLM call, so treat the figure as a range.
Rl0007
force-pushed
the
feat/rag-chat-lancedb
branch
from
August 11, 2026 03:20
53ac237 to
59e6ce2
Compare
Twelve images covering the Ask flow with citations, page/line provenance, the cost meter, the naive-vs-routed comparison, the refusal state, a rendered mermaid diagram, the reconstructed ICAI surcharge tables, the page verdict list, and three mobile views plus dark mode.
Three defects found by testing the Ask path against two real ICAI documents, plus the cleanup pass that followed. Reranking — answers were being refused that the corpus covered - `search()` re-sorted by fusion score after reranking and then applied the limit, so the reranked winner was pushed back to its fusion position and sliced off. "Slab rates under section 115BAC(1A)" scored 8.0 at fusion rank 10 and never reached the answer; the question was refused. One `rank_key` now drives both the rerank and the slice. - Grading 35 candidates in one call makes the classifier emit a run of zeros. Batching at 10 returns real scores. A batch whose reply skips candidates is dropped whole rather than letting the missing ones read as a confident 0. - Refusal now needs two legs to agree: a below-floor rerank is overruled by a strong embedding match. Calibrated on both corpora — answerable questions peak 0.56-0.63, unanswerable 0.39-0.45. Genuine refusals still refuse. - When the embedding leg overrules, citations carry `rerank_score: null` rather than 0.0, so the UI stops showing a dead relevance chip. Cost — the reported figure was ~15x too low Only the price printed on the response body was counted. The router and reranker use the REST client, which returns cost inline; synthesis goes through litellm, which reports price to a callback on its own thread. So the meter added $0.00 for the leg carrying ~92% of the spend. A `CustomLogger` keyed on `litellm_call_id` now bills each call back to the collector that started it, and the second accounting mechanism in `history.py` is deleted — session totals are summed from message rows so the header cannot drift. Verified against OpenRouter's own generation records: an ICAI question costs $0.047, not the $0.003 previously reported. Efficiency - Rerank batches run concurrently. `frappe.local` is unbound on a pool thread, so the API key is resolved on the calling thread and passed in. Measured 19.81s -> 16.00s median per ask, with identical call count and spend. - `index_status` did one LanceDB connect and one full scan per readable project; now one scan with a single `project IN (...)`. - Page propagation rebuilt the whole FTS index once per section and re-read every page of the document each time; now one batched upsert and one FTS rebuild, loading only the relevant pages. - Sectionising fired the reindex hook per row (~592 redis round trips, 295 of them no-ops); it now suspends per-row indexing and queues one rebuild. - `get_page_image()` ran per page inside the remediation loop; the field is read with the rest of the row. - Mermaid sources were parsed up to nine times per page; now once. - `find_regions` read each page's drawings twice. Reuse and dead code Duplicate `get_pages_by_document` / batched-IN / page-line-span helpers collapsed to one definition each; `useIsMobile` folded into `useMediaQuery` so the app has one media-query registry; AskWiki reuses `MarkdownPreview`, which also gives Ask answers mermaid rendering they never had; the eval harness now calls production's `retrieve()` and `compare()` instead of copying them, which is the drift it exists to catch. Removed an unreachable `EvalScoreboard` branch, a dead `fts` relevance basis, an unused `page_regions` parameter, and a `wait_for_prices` call that could add 5s to a failing ask without affecting any reported number. Streaming answers no longer re-parse the whole markdown per token. References to five spec files that are not in this repo are rewritten to point at the code that holds the information.
Asked "how many job descriptions are in the Demo Corpus project?", the agent
answered "there are none". There are 15. Two faults produced the same sentence,
so a correct chain of reasoning ended in a false statement.
- The `query` argument was a raw substring filter over title and hierarchy
path. Questions are plural ("job descriptions"), titles are singular
("Job Description — Theatre Scrub Practitioner"), so every match was
filtered away. It is now a narrowing hint: token-wise, normalised for case,
punctuation and trailing plurals, and when it matches nothing the full set
is returned with the filter reported as ignored rather than as absence.
- The model passed the project title, not its id. `Ctx.default_document`
already guarded against exactly this, with a comment noting models echo
display labels instead of bare ids; the project field had no such guard.
Added `Ctx.default_project`, and closed the same hole in `semantic_search`.
Three outcomes that were indistinguishable now read differently: an unknown
section type (with the near matches, and an explicit note that this is not
evidence the content is missing), a valid type that is empty in scope (naming
the scope and which types do have content), and a filter that removed
everything (naming how many it removed). Every reply carries the in-scope
count and the resolved scope, so absence cannot be inferred from silence.
Also closes a fixture leak: types created by tests that drive the agent loop
survived rollback because the loop commits mid-turn, and were being offered to
users as real taxonomy suggestions.
OpenRouter routes by price by default and re-draws a provider per call, so concurrent rerank batches were scattering across endpoints of different speed and the wall clock was set by the slowest draw. Pinning the endpoint removes the draw. Measured over 5 batches, 9 runs each: unpinned 7.8s median with 1.9x effective concurrency and a 24.7s worst case; pinned 2.6s median, 4.2x, 4.1s worst case. The latency was the smaller half. Six of eighteen unpinned runs returned truncated JSON, which discards the whole rerank verdict and drops the answer back to fusion order — against zero of eighteen pinned. That is the most likely source of the intermittent all-zero rerank we chased earlier, and it means provider scatter was corrupting results, not just slowing them. `allow_fallbacks` stays on: a reranker that cannot reach its preferred provider must degrade to a slower one, never fail. Retrieval with reranking measures 8.9s to 5.8s end to end. Total ask time is unchanged in a single sample because the router leg drew slowly on that run; the router change here is the same one-line pin, so that reading needs repeating before anything is concluded from it. Also raises the rerank worker cap now that batches no longer contend for a provider draw.
test_ask_cost pins the per-ask figure against the sum of its individual LLM calls, and the session total against the sum of its message rows. Without it the 15x undercount could return silently: litellm reports synthesis price via a callback rather than on the response, so the leg carrying ~92% of the spend read as $0.00. docs/rag-latency-prior-art.md records what production systems do about the same problems, with sources and dates. Its main finding contradicts the assumption we started from: synthesis is decode-bound, so trimming the 55k character prompt is a cost fix rather than a speed fix, and the reranker's model is the bottleneck rather than its depth.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a retrieval layer and an Ask/RAG Lab UI over parsed documents, and fixes several pipeline defects found while running two real ICAI study PDFs through it.
Why
Wikify could turn PDFs into a browsable wiki, but not answer questions about them. The motivating case is "give me all the job descriptions across all the PDFs" — an exhaustive intent that similarity search answers badly, because top-k has no idea whether the right answer is 3 items or 300.
Measured on the demo corpus: naive top-8 vector search returns 8 sections across 4 of 5 documents (6 correct). A metadata filter returns all 15 across 5 of 5. Naive silently misses 9 sections and a whole document.
Retrieval —
wikify/rag/vector,fts,hybrid(RRF-fused), andfilter— which returns every match with no top-k truncation.document > hierarchy pathprefix (kept out of displayed text). Free here, because the section tree already existed.Answering
(+)to(-)previously passed a 0.85 similarity check — for exam prep, a citation that renders as confirmed while misstating a rate is worse than none.Wikify Ask Session/Wikify Ask Message.Pipeline fixes (found on real documents)
chars < 250 AND drawings > 40# TIE TIT Moloat composite 0.058search()re-sorted by fusion score after reranking, then slicedPerformance
Measured A/B on the same question, three runs each:
index_statusindex scansRerank batches now run concurrently —
frappe.localis unbound on a pool thread, so the API key is resolved on the calling thread and passed in. Call count and spend are identical to the cent.Frontend
/askand/rag-lab: sources render before the answer, citation chips, page/line provenance, and a naive-vs-routed comparison showing what similarity search missed. Unranked results no longer draw a meaningless full score bar.Mobile across the app: shell navigation, drill-downs replacing desktop splits, and graph views that degrade to a grouped list rather than an unreadable canvas.
Evaluation
12 golden questions scoring recall, precision and completeness, plus a self-contained HTML scorecard. Routed retrieval beats naive 66% to 87-90% recall on the demo corpus. Routing is an LLM call, so that is a range, not a point.
Known open items
Being explicit rather than presenting this as finished. These were surfaced by a structured review of this branch and are not fixed here:
sessionis two different identifiers sharing one name. The frontend mints a realtime correlation token; the backend treats it as aWikify Ask Sessiondocname. So every ask opens a new session, conversation history never replays, andhas_permissionon a non-existent name raisesDoesNotExistErrorfor non-Administrators — the first non-admin user to ask a question gets a 404. Administrator short-circuits the check, which is why testing missed it. Needs the field split intostreamandsession.Source Page.on_updatefires for almost nothing in production, because every real write goes throughfrappe.db.set_value. Invalidation belongs inengine/store.py, which already calls itself the write funnel.replace_sectionsis not atomic. The title clip removed one trigger, not the hazard: delete-then-insert with no savepoint, and the failure handler commits the partial tree and marks the importReview.set_canonical_markdowndoes not invalidate the verdict, so an agent edit can leave "pass 0.99" on text nobody scored.test_g1) pins a naivecorrect_countthat moved when the corpus was re-sectioned.Testing
test_rag_core(28),test_rag_api(38),test_evidence(32),test_diagrams(21),test_regions(8),test_sectionize(23),test_ask_cost(6),test_ask_history(16),test_page_propagation(13),test_remediate_adoption(12),test_canonical_verdict(4), plus the pre-existing suites for blast radius. Verified end to end againstwikify.localhostwith two real 180- and 236-page ICAI documents.