diff --git a/README.md b/README.md index e0775eb..680377e 100644 --- a/README.md +++ b/README.md @@ -87,8 +87,10 @@ Answers (through `/v1/fetch` or `/v2/scrape`): - `--question "…"` for a **digest** that, on a PDF, cites each fact as `(p. N)`. A range returns at most 100 pages; grep shows at most 100 matches and takes a pattern of at most -200 bytes. `pages` and `grep` on anything but a PDF are a `400`. A question over a document too long -to digest (more than 200 chunks) is refused with +200 bytes. `pages` and `grep` on anything but a PDF are a `400`. A question on a long document is +answered first from BM25 (`retrieval: "bm25"`) — lexical, before the document's chunks are +embedded in the background — and hybrid on later questions, once those embeddings exist. A question +over a document too long to answer even by retrieval (more than 5000 chunks) is refused with `422 document too long for question (N chunks); use grep or pages`. Scanned PDFs have no text layer and return `no text layer (scanned PDF?)`; point them at an OCR diff --git a/docs/implementer-log.md b/docs/implementer-log.md index db8dfed..f6fcb3b 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -14,6 +14,7 @@ One row per task session that ends in a commit. Newest last. | p5b/01-splitpages-bm25 | 2026-09-26 | done | 1 | gate: ok | none | `splitpages.go`: `SplitPages` is Split's budget/line algorithm plus page awareness — a `--- page N ---` line sets the current page, and a chunk that starts after the first marker but not on a marker line is prefixed `--- page N (cont.) ---\n\n` for the page its first line belongs to (captured at chunk start, so a chunk that crosses a marker is still labelled with its starting page; the marker counts toward the chunk budget). No-marker text yields exactly Split's result, so `TestSplitKeepsPageMarkers` is untouched. `bm25.go`: `BM25` with k1=1.2/b=0.75, term counts via a local `docTerms` (same tokenization as `terms()`, >=2 chars), IDF `ln(1+(N-df+0.5)/(df+0.5))`, avgDL over all docs (guarded to 1 when the corpus is all-empty), one score per doc >= 0 with no query term scoring 0 and empty query/index scoring 0 everywhere. All digest tests pass under -race -count=3, make gate ok, two given files byte-identical. | | p5b/02-retrieval | 2026-09-26 | done | 1 | gate: ok | none | Cached chunk embeddings + BM25-first retrieval. `digest.go`: `EmbedSyncMax=48`, `MaxDocChunks=5000`; `Result` gains `Retrieval string` and `Embedded int`; `selectText` now returns `(body, used, total, mode, embedded, err)` and `Run` sets both; `PutDigest` skips storing any `mode=="bm25"` answer (provisional, not cached). `retrieval.go`: `hybridOrBM25` — no cache, `len(chunks)<=48` embeds question+chunks inline and scores hybrid; `>48` scores BM25 and queues; cache present embeds the question and only the uncached chunks inline and scores hybrid on the union; both embed paths return hybrid, both bm25 paths return `bm25`. `background.go`: `bgWorker` with `bgOnce sync.Once` (creates the worker, never starts a goroutine) + `w.started` under `w.mu` (starts the goroutine once), dedup by `contentHash|callerClass`, full-queue drops the doc without marking seen so a later question re-queues, `acquireGate` retries once after 1 s on refusal, `WaitBackground` polls every 20 ms for true idle. `retrieval_test.go` copied byte-identical from the plan. All 12 digest tests pass under `-race -count=3`; `make gate` ok. | | p5a/07-api-cache-pages | 2026-09-26 | done | 1 | gate: ok | none | Implemented the PDF document cache with revalidation in `doFetch`: `pdfFromCache` (miss / fresh within `PDFTTL` / stale → `FetchIf` with 304→`TouchPDF`+answer-fresh / 200→return response with ok=false and no second GET) and `storePDF` after `extractPDF` (`ContentHash=res.SHA256`, ETag/Last-Modified from headers, FetchedAt=now, Meta=JSON of `doc.Meta`, Pages=`doc.Text`). `pages`/`grep` passed into `pdfdoc.Options` in `renderPDF`; `ErrBadPages`/`ErrBadGrep`→400 with the error's own text, non-PDF→400 "pages and grep apply to PDFs only" before the doc block. Question limit in digest mode: `n=len(digest.Split(pdfdoc.Full(doc), ChunkTokens))`, `n>PDFMaxQuestionChunks`→422 "document too long for question (n chunks); use grep or pages" before any digest call. Envelope `cached` set true for cache answers via the synthetic cache res's empty body (`len(res.Body)==0`); the one `doFetch` change beyond the rule-1 call was reusing that 200 response instead of a second `Fetch`. Added `fmt` import to api.go. Given file byte-identical; make gate ok. | +| p5b/03-api | 2026-09-26 | done | 1 | gate: ok | none | The digest now answers long documents itself (task 02), so the API's refusal limit is only a safety cap: `DefaultPDFMaxQuestionChunks` 200→5000 in `internal/api/pdf.go` (the 422 message text unchanged, still "document too long for question (N chunks); use grep or pages"). In `v1fetch` (`internal/api/api.go`) the digest-meta map gains `"retrieval": dr.Retrieval` and `"embedded": dr.Embedded` next to `chunks_used`/`chunks_total`, only when the digest ran without a fallback. Copied the two given tests byte-identical from the plan — the replacement `pdf_test.go` is now the protected one and asserts `DefaultPDFMaxQuestionChunks == 5000`; `pdf_retrieval_test.go` asserts the first long-doc question returns `retrieval=bm25`, `embedded=0`. README PDF section: a long-doc question is answered first from BM25 (`retrieval: "bm25"`) while its chunks embed in the background and hybrid on later questions, plus the 5000-chunk cap. One sentence added to `skill/SKILL.md`. All api tests pass under `-race -count=3`; make gate ok; given files byte-identical. | ## P5a review (owner, 2026-09-26) diff --git a/internal/api/api.go b/internal/api/api.go index 715a057..a9cfe46 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -569,6 +569,8 @@ func (s *Server) v1fetch(w http.ResponseWriter, r *http.Request) { "model": dr.Model, "chunks_used": dr.ChunksUsed, "chunks_total": dr.ChunksTotal, + "retrieval": dr.Retrieval, + "embedded": dr.Embedded, "fallback": false, "cached": dr.Cached, } diff --git a/internal/api/pdf.go b/internal/api/pdf.go index 7c7179f..d13e313 100644 --- a/internal/api/pdf.go +++ b/internal/api/pdf.go @@ -20,9 +20,11 @@ import ( const ( // DefaultPDFTTL is how long a cached PDF is served before it is revalidated. DefaultPDFTTL = 24 * time.Hour - // DefaultPDFMaxQuestionChunks is the longest document (in digest chunks) a question is - // answered over; a longer one is refused in favour of grep or pages. - DefaultPDFMaxQuestionChunks = 200 + // DefaultPDFMaxQuestionChunks is the safety cap on how long a document a question is + // answered over; beyond it the question is refused in favour of grep or pages. Longer + // documents are handled by retrieval (BM25 first, embedding their chunks in the + // background), so this only bounds the un-answerable. + DefaultPDFMaxQuestionChunks = 5000 ) // runner returns the configured extraction child, or a zero Runner that runs diff --git a/internal/api/pdf_retrieval_test.go b/internal/api/pdf_retrieval_test.go new file mode 100644 index 0000000..2793160 --- /dev/null +++ b/internal/api/pdf_retrieval_test.go @@ -0,0 +1,75 @@ +package api + +// P5b task 03: a question on a long PDF is answered (by BM25 first) instead of refused, and the +// envelope says how the digest was built. + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "git.wntrmute.dev/kyle/sift/internal/digest" + "git.wntrmute.dev/kyle/sift/internal/pdftest" +) + +// fakeModel answers /embed with a fixed vector and /chat with a one-word SSE reply. +func fakeModel(t *testing.T) *httptest.Server { + mux := http.NewServeMux() + mux.HandleFunc("/embed", func(w http.ResponseWriter, r *http.Request) { + var body struct { + Input []string `json:"input"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + var data []map[string]any + for i := range body.Input { + data = append(data, map[string]any{"index": i, "embedding": []float64{1, float64(i % 7)}}) + } + _ = json.NewEncoder(w).Encode(map[string]any{"data": data}) + }) + mux.HandleFunc("/chat", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/event-stream") + _, _ = w.Write([]byte(`data: {"choices":[{"delta":{"content":"answer (p. 3)"}}]}` + "\n\ndata: [DONE]\n\n")) + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv +} + +func TestPDFLongQuestionIsAnswered(t *testing.T) { + r := newPDFRig(t) + m := fakeModel(t) + db := r.s.Digest.Cache + r.s.Digest = &digest.Engine{ChatURL: m.URL + "/chat", EmbedURL: m.URL + "/embed", Cache: db, Gate: digest.NewGate(2, 8, 5*time.Second), Client: m.Client()} + var pages []string + for p := 1; p <= 260; p++ { // ~260 pages × 40 lines: well over EmbedSyncMax chunks + pages = append(pages, pdftest.Lorem(p, 40)) + } + o := newOrigin(t, "application/pdf", pdftest.Build(pdftest.Doc{Title: "Long", Pages: pages})) + code, out, _ := r.fetch(t, r.fleet, map[string]any{"url": o.srv.URL, "question": "what is on page three?", "context": false}) + if code != 200 { + t.Fatalf("long question: %d %v, want 200 (P5a refused this)", code, out) + } + dg, _ := out["digest"].(map[string]any) + if dg == nil || dg["retrieval"] != "bm25" || dg["embedded"] != 0.0 { + t.Errorf("digest meta = %v, want retrieval bm25 and embedded 0 on the first question", dg) + } + if c, _ := out["content"].(string); !strings.Contains(c, "answer") { + t.Errorf("content %q", c) + } +} + +func TestPDFSmallQuestionReportsRetrieval(t *testing.T) { + r := newPDFRig(t) + m := fakeModel(t) + db := r.s.Digest.Cache + r.s.Digest = &digest.Engine{ChatURL: m.URL + "/chat", EmbedURL: m.URL + "/embed", Cache: db, Gate: digest.NewGate(2, 8, 5*time.Second), Client: m.Client()} + o := newOrigin(t, "application/pdf", threePDF()) + code, out, _ := r.fetch(t, r.fleet, map[string]any{"url": o.srv.URL, "question": "what is on page two?", "context": false}) + dg, _ := out["digest"].(map[string]any) + if code != 200 || dg == nil || dg["retrieval"] != "whole" { + t.Errorf("small document: %d digest %v, want retrieval whole", code, dg) + } +} diff --git a/internal/api/pdf_test.go b/internal/api/pdf_test.go index 041978a..2874eea 100644 --- a/internal/api/pdf_test.go +++ b/internal/api/pdf_test.go @@ -396,7 +396,8 @@ func TestPDFQuestionTooLong(t *testing.T) { if msg, _ := out["error"].(string); code != 422 || !strings.Contains(msg, "too long") || !strings.Contains(msg, "grep") { t.Errorf("long question: %d %v; want 422 … too long … grep", code, out) } - if DefaultPDFMaxQuestionChunks != 200 { - t.Errorf("DefaultPDFMaxQuestionChunks = %d, want 200", DefaultPDFMaxQuestionChunks) + // P5b: long documents are answered by retrieval now; the limit is only a safety cap. + if DefaultPDFMaxQuestionChunks != 5000 { + t.Errorf("DefaultPDFMaxQuestionChunks = %d, want 5000", DefaultPDFMaxQuestionChunks) } } diff --git a/skill/SKILL.md b/skill/SKILL.md index 4a0fe97..7ea19c4 100644 --- a/skill/SKILL.md +++ b/skill/SKILL.md @@ -61,7 +61,8 @@ PDFs work: `sift fetch` extracts them and returns the text. A short one comes ba long one comes back as a map — an outline with page numbers, then the first pages. Then: - `sift fetch URL --pages 41-60` for a page range, `--grep 'pattern'` for matching lines, or - `--question "…"` for a digest that cites each fact as `(p. N)`. + `--question "…"` for a digest that cites each fact as `(p. N)`; `--question` works on long + PDFs, and the first answer may be `bm25` (lexical) — ask again later for a sharper one. - Scanned PDFs have no text layer: sift reports `no text layer (scanned PDF?)` — point it at an OCR source rather than trying to fix it here. - A range returns at most 100 pages and grep at most 100 matches. On a very long document