P5b and P5c plans: long-document retrieval (BM25 first, background embedding) and /v1/extract + sift extract; given tests

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs/plans/p5c/01-extract-api.md

@@ -0,0 +1,76 @@
+# P5c task 01: `POST /v1/extract`
+
+**Branch:** `p5c` (`git switch -c p5c master` if it does not exist, else `git switch p5c`; `git status --short` must be empty, otherwise stop)
+**Commit subject:** `api: POST /v1/extract answers an uploaded PDF like a fetched one (content-addressed cache)`
+
+## Goal
+
+`POST /v1/extract` with a PDF as the request body returns the `/v1/fetch` PDF envelope:
+text with page markers or the map, `pages`, `grep`, or a cited digest. Nothing is fetched.
+The bytes are hashed, the cache key is the hash, and the same file is extracted once.
+
+## Files
+
+- Copy: `internal/api/extract_test.go`
+- Create: `internal/api/extract.go` (the handler and its envelope)
+- Modify: `internal/api/api.go`: only the route in `New`, the `Server` field and constant
+  below, and the two lines in `ServeHTTP` (rule 1); `docs/implementer-log.md`
+
+## Interfaces
+
+```go
+package api
+
+const DefaultPDFMaxUpload = 100 << 20
+
+// in Server:
+	PDFMaxUpload int64 // 0 = DefaultPDFMaxUpload
+
+// route: s.Mux.HandleFunc("POST /v1/extract", s.v1extract)
+// query parameters (all optional): pages, grep, question, mode ("clean" | "digest";
+// default digest when question is set, else clean)
+```
+
+What exists and is reused, unchanged:
+- `s.caller`, `s.callerClass`;
+- `s.extractPDF(ctx, c, *fetch.Resource)`, `pdfError`, `s.renderPDF`, `s.storePDF(key, res, doc)`, `s.pdfFromEntry`;
+- `s.cache()`, `dcache.PDFKey`, `clean.Kind`, `pdfdoc.Full`;
+- `s.Digest.Run`, `s.Prom.PDFCacheHit`, `writeErr`, `writeJSON`.
+
+## Rules
+
+1. **`ServeHTTP`:**
+   - today it wraps every request body in `http.MaxBytesReader(w, r.Body, maxReqBody)` (64 KiB). For `/v1/extract` use `PDFMaxUpload` instead;
+   - add `/v1/extract` to the paths that get `requestDeadline`;
+   - every other path keeps 64 KiB (the given test checks that).
+2. **Handler:**
+   - Authenticate as the other endpoints do.
+   - Read the whole body. Empty → `400 empty body`. Over the limit (`*http.MaxBytesError`) → `413 too large (>N MB)`.
+   - `clean.Kind(Content-Type, body)` must be `"pdf"`; otherwise `415 extract accepts PDFs only`.
+3. **Cache:**
+   - `hash := "sha256:" + hex(sha256(body))`, `key := dcache.PDFKey("upload:"+hash, s.callerClass(c))`.
+   - An entry → the doc from it, `cached: true`, `PDFCacheHit`, no child. The content is addressed by its hash, so there is **no TTL and no revalidation**.
+   - Otherwise build `&fetch.Resource{URL: "upload", FinalURL: "upload", Status: 200, ContentType: ct, Body: body, Hash: hash, FetchedAt: now}`, then `extractPDF`, then `storePDF(key, res, doc)`.
+   - Errors go through `pdfError`, via the existing `writeFetchErr` (it adds `Retry-After`).
+4. **Answer:** the same fields as `/v1/fetch` for a PDF:
+   - `content`, `mode`, `untrusted: true`, `warnings`, `sizes`, `timings`, `pdf`, and `digest` when a digest ran, including P5b's `retrieval` and `embedded`;
+   - `source` is `{"kind": "upload", "hash", "bytes", "cached", "fetched_at"}`, with **no `url`** key;
+   - bad `pages`/`grep` → 400 (as in `v1fetch`); the question limit → the same 422 as `v1fetch`.
+   - Do not call Honcho context for uploads.
+5. **Logging:** one line, `"extract"`, with caller, bytes, `pdf_pages`, mode and cached. Never a title or any text. The server never sees a filename.
+
+## Steps
+
+- [ ] **1.** Branch; copy the given test. **2. See it fail.**
+- [ ] **3.** `api.go` hooks, **4.** `extract.go`.
+- [ ] **5.** `gofmt -w`; `go test -race -count=3 ./internal/api/` → `ok`. **6.** `make gate`.
+- [ ] **7.** Row `p5c/01-extract-api`; commit.
+
+```sh
+git add internal/api docs/implementer-log.md
+git commit -F .state/commit-msg.txt
+```
+
+## Done when
+
+- All tests pass under `-race -count=3`; `make gate` ok; given file byte-identical.

docs/plans/p5c/02-extract-cli.md

@@ -0,0 +1,60 @@
+# P5c task 02: `sift extract <file.pdf>`
+
+**Branch:** `p5c` (`git switch p5c`; `git status --short` must be empty, otherwise stop)
+**Commit subject:** `cli: sift extract uploads a local PDF; README and skill`
+
+## Goal
+
+`sift extract thesis.pdf [--pages 41-60] [--grep RE] [--question Q] [--json]` uploads the file
+to `/v1/extract` and prints the answer exactly as `sift fetch` does.
+
+## Files
+
+- Copy: `cmd/sift/extract_test.go`
+- Create: `cmd/sift/extractcmd.go`
+- Modify: `cmd/sift/main.go` (a `case "extract":` and one help line), `README.md`,
+  `skill/SKILL.md`, `docs/implementer-log.md`
+
+## Interfaces
+
+```go
+package main
+
+type extractOpts struct {
+	File, Pages, Grep, Question, Mode string
+	JSON                              bool
+}
+
+const extractUsage = "usage: sift extract <file.pdf> [--pages a-b] [--grep RE] [--question Q] [--mode clean|digest] [--json]"
+
+func parseExtractOpts(args []string) (extractOpts, error) // flags before or after the file, as fetch
+func extractURL(base string, o extractOpts) string        // base + "/v1/extract?" + only the options that are set; mode=digest when Question is set and Mode is empty
+```
+
+## Rules
+
+1. The command:
+   - Resolve the base URL and key exactly as `sift fetch` does (reuse its client-config helpers; do not copy them).
+   - `stat` the file: over 100 MiB → exit with a message before uploading.
+   - `POST` the bytes with `Content-Type: application/pdf` and the bearer key.
+   - Print `content` (or the whole envelope with `--json`), with the same errors and exit codes as fetch (`exitFor`).
+   - Do not read the whole file into memory twice: stream it with `os.Open` as the request body and set `ContentLength`.
+2. **`README.md`:** a `sift extract` subsection next to `sift fetch`.
+3. **`skill/SKILL.md`:** "A PDF on disk (an attachment, a download): `sift extract <file>`, with the same `--pages`/`--grep`/`--question`. Never run `pdftotext` yourself."
+
+## Steps
+
+- [ ] **1.** `git switch p5c`; copy the given test. **2. See it fail.**
+- [ ] **3.** `extractcmd.go`, `main.go`, README, skill.
+- [ ] **4.** `gofmt -w`; `go test -race -count=3 ./cmd/sift/` → `ok`. **5.** `make gate`.
+- [ ] **6.** `make build && bin/sift extract 2>&1 | head -1` prints the usage line.
+- [ ] **7.** Row `p5c/02-extract-cli`; commit.
+
+```sh
+git add cmd/sift README.md skill/SKILL.md docs/implementer-log.md
+git commit -F .state/commit-msg.txt
+```
+
+## Done when
+
+- All tests pass; `make gate` ok; given file byte-identical.

docs/plans/p5c/README.md

@@ -0,0 +1,21 @@
+# P5c implementation plan: PDFs already on a host
+
+> **For the implementing model:** do not work from this file. The owner gives you one task file
+> at a time. This file is the index for the owner and the reviewer.
+
+**Goal:** an agent with a PDF on disk (a Mattermost attachment, a download) gets the same
+answer as for a PDF URL — text with page markers, the map, pages, grep, cited digests — by
+uploading the bytes: `POST /v1/extract`, CLI `sift extract <file>`. No poppler on the host, no
+shell `pdftotext`. The cache key is the content hash, so the same file is extracted once.
+
+| Task | Package | Given |
+|---|---|---|
+| 01-extract-api | `internal/api` | `api/extract_test.go` |
+| 02-extract-cli | `cmd/sift`, `README.md`, `skill/SKILL.md` | `cmd/sift/extract_test.go` |
+
+Runs after P5b (task 01 uses the P5b digest fields). Given tests compiled against a panic-only
+skeleton on master `1311cf3` and failed for the intended reasons.
+
+## Changes during the run
+
+(none yet)

docs/plans/p5c/_files/cmd/sift/extract_test.go

@@ -0,0 +1,46 @@
+package main
+
+// P5c task 02: `sift extract <file>` uploads a PDF to /v1/extract.
+
+import (
+	"net/url"
+	"strings"
+	"testing"
+)
+
+func TestParseExtractOpts(t *testing.T) {
+	o, err := parseExtractOpts([]string{"thesis.pdf", "--pages", "41-60", "--grep", "attention head"})
+	if err != nil || o.File != "thesis.pdf" || o.Pages != "41-60" || o.Grep != "attention head" {
+		t.Errorf("%+v %v", o, err)
+	}
+	o, err = parseExtractOpts([]string{"--question", "what is the method?", "--json", "paper.pdf"})
+	if err != nil || o.File != "paper.pdf" || o.Question != "what is the method?" || !o.JSON {
+		t.Errorf("flags first: %+v %v", o, err)
+	}
+	if _, err := parseExtractOpts(nil); err == nil {
+		t.Error("no file: want an error")
+	}
+	if _, err := parseExtractOpts([]string{"a.pdf", "b.pdf"}); err == nil {
+		t.Error("two files: want an error")
+	}
+	if !strings.Contains(extractUsage, "sift extract <file.pdf>") {
+		t.Errorf("usage %q", extractUsage)
+	}
+}
+
+func TestExtractURL(t *testing.T) {
+	u, err := url.Parse(extractURL("https://sift.example:8443", extractOpts{File: "x.pdf", Pages: "2-3", Grep: "a b&c"}))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if u.Path != "/v1/extract" || u.Query().Get("pages") != "2-3" || u.Query().Get("grep") != "a b&c" {
+		t.Errorf("url %s", u)
+	}
+	if u.Query().Has("question") || u.Query().Has("mode") {
+		t.Errorf("unset options must not be sent: %s", u)
+	}
+	u, _ = url.Parse(extractURL("https://sift.example:8443", extractOpts{File: "x.pdf", Question: "why?"}))
+	if u.Query().Get("question") != "why?" || u.Query().Get("mode") != "digest" {
+		t.Errorf("question url %s (mode must be digest with a question)", u)
+	}
+}

docs/plans/p5c/_files/internal/api/extract_test.go

@@ -0,0 +1,122 @@
+package api
+
+// P5c task 01: POST /v1/extract takes a PDF's bytes (a file already on the caller's host), and
+// answers exactly like /v1/fetch does for a PDF URL. The cache key is the content hash.
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+func (r *pdfRig) extract(t *testing.T, key, query string, body []byte, ct string) (int, map[string]any) {
+	t.Helper()
+	req := httptest.NewRequest(http.MethodPost, "/v1/extract"+query, bytes.NewReader(body))
+	if ct != "" {
+		req.Header.Set("Content-Type", ct)
+	}
+	if key != "" {
+		req.Header.Set("Authorization", "Bearer "+key)
+	}
+	rec := httptest.NewRecorder()
+	r.s.ServeHTTP(rec, req)
+	var out map[string]any
+	_ = json.Unmarshal(rec.Body.Bytes(), &out)
+	return rec.Code, out
+}
+
+func TestExtractUpload(t *testing.T) {
+	r := newPDFRig(t)
+	pdf := threePDF()
+	code, out := r.extract(t, r.fleet, "", pdf, "application/pdf")
+	if code != 200 {
+		t.Fatalf("%d %v", code, out)
+	}
+	content, _ := out["content"].(string)
+	if !strings.Contains(content, "--- page 2 ---") || !strings.Contains(content, "quick brown fox") {
+		t.Errorf("content %q", content)
+	}
+	if p := pdfInfo(t, out); p["pages"] != 3.0 || p["title"] != "Three Pages" {
+		t.Errorf("pdf %v", p)
+	}
+	src, _ := out["source"].(map[string]any)
+	if src["kind"] != "upload" || src["bytes"] != float64(len(pdf)) || !strings.HasPrefix(fmt.Sprint(src["hash"]), "sha256:") || src["cached"] != false {
+		t.Errorf("source %v", src)
+	}
+	if _, hasURL := src["url"]; hasURL {
+		t.Error("an upload has no URL; source must not invent one")
+	}
+	// The same bytes again: from the cache, no child.
+	code, out = r.extract(t, r.fleet, "?pages=2", pdf, "application/octet-stream")
+	if code != 200 || r.runs() != 1 {
+		t.Fatalf("second upload: %d, %d extractions; want 200 and 1", code, r.runs())
+	}
+	if src, _ := out["source"].(map[string]any); src["cached"] != true {
+		t.Errorf("second upload source %v, want cached", src)
+	}
+	if c, _ := out["content"].(string); strings.Contains(c, "--- page 1 ---") || !strings.Contains(c, "--- page 2 ---") {
+		t.Errorf("pages=2: %q", c)
+	}
+	code, out = r.extract(t, r.fleet, "?grep=quick+brown", pdf, "application/pdf")
+	if c, _ := out["content"].(string); code != 200 || !strings.Contains(c, "p. 2: The quick brown fox") {
+		t.Errorf("grep: %d %q", code, c)
+	}
+}
+
+func TestExtractRefusals(t *testing.T) {
+	r := newPDFRig(t)
+	if code, _ := r.extract(t, "", "", threePDF(), "application/pdf"); code != 401 {
+		t.Errorf("no key: %d, want 401", code)
+	}
+	if code, out := r.extract(t, r.fleet, "", []byte("<html>not a pdf</html>"), "text/html"); code != 415 {
+		t.Errorf("html body: %d %v, want 415", code, out)
+	}
+	if code, _ := r.extract(t, r.fleet, "", nil, "application/pdf"); code != 400 {
+		t.Errorf("empty body: %d, want 400", code)
+	}
+	if code, _ := r.extract(t, r.fleet, "?pages=0-1", threePDF(), "application/pdf"); code != 400 {
+		t.Errorf("bad pages: %d, want 400", code)
+	}
+	r.s.PDFMaxUpload = 1000
+	if code, out := r.extract(t, r.fleet, "", threePDF(), "application/pdf"); code != 413 {
+		t.Errorf("over PDFMaxUpload: %d %v, want 413", code, out)
+	}
+	if DefaultPDFMaxUpload != 100<<20 {
+		t.Errorf("DefaultPDFMaxUpload = %d, want 100 MiB", DefaultPDFMaxUpload)
+	}
+	// The ordinary JSON endpoints keep their small request limit.
+	big := bytes.Repeat([]byte("x"), 128<<10)
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequest(http.MethodPost, "/v1/fetch", bytes.NewReader(append([]byte(`{"url":"`), append(big, []byte(`"}`)...)...)))
+	req.Header.Set("Authorization", "Bearer "+r.fleet)
+	r.s.ServeHTTP(rec, req)
+	if rec.Code == 200 {
+		t.Error("a 128 KiB /v1/fetch body was accepted; the JSON limit must still apply there")
+	}
+}
+
+func TestExtractRestrictedPartition(t *testing.T) {
+	r := newPDFRig(t)
+	pdf := threePDF()
+	if code, _ := r.extract(t, r.gs, "", pdf, "application/pdf"); code != 200 {
+		t.Fatalf("restricted upload: %d", code)
+	}
+	r.logs.mu.Lock()
+	for _, rec := range r.logs.recs {
+		var b strings.Builder
+		b.WriteString(rec.Message)
+		rec.Attrs(func(a slog.Attr) bool { fmt.Fprintf(&b, " %s=%v", a.Key, a.Value); return true })
+		if strings.Contains(b.String(), "Three Pages") || strings.Contains(b.String(), "quick brown") {
+			t.Errorf("an upload's log line carries document text: %q", b.String())
+		}
+	}
+	r.logs.mu.Unlock()
+	if code, _ := r.extract(t, r.fleet, "", pdf, "application/pdf"); code != 200 || r.runs() != 2 {
+		t.Errorf("fleet after restricted: %d, %d extractions; want 200 and 2 (separate partitions)", code, r.runs())
+	}
+}