docs/plans/p5a: 01 kind+fetch limits, 02 pdftext child + runner, 03 dcache PDF tables, 04 pdfpool, 05 pdfdoc render (map/pages/grep), 06 api, 07 digest citations, metrics, CLI, skill, unit, README. Given tests compile against a panic-only skeleton and fail there for the intended reasons; PDFs for tests are built by internal/pdftest (given), plus two qpdf-encrypted fixtures. Owner changes for the run: vendor go-pdfkit/extract v0.3.0 + reader v0.6.0 (pinned via tools.go until task 02); AGENTS.md, tools/run-plan.sh (enforces a Co-Authored-By trailer for the model), docs/implementer-log.md, `make gate`; gofmt (whitespace) on clean.go and guard; TestUnsupportedType uses a zip now that PDFs are supported; requestDeadline 90→210 s and writeTimeout 120→240 s for the PDF path's worst case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
151 lines
7.0 KiB
Markdown
151 lines
7.0 KiB
Markdown
# P5a task 06: PDFs through `/v1/fetch` and `/v2/scrape`
|
||
|
||
**Branch:** `p5a` (`git switch p5a`; `git status --short` must be empty, otherwise stop)
|
||
**Commit subject:** `api: serve PDFs — child extraction via the pool, document cache, map, pages, grep`
|
||
|
||
## Goal
|
||
|
||
Wire tasks 01–05 into the API. A PDF URL fetched through `/v1/fetch` or `/v2/scrape` is:
|
||
1. extracted once in the child, under the pool;
|
||
2. cached per page;
|
||
3. answered with the whole text, the map, a page range, or grep results.
|
||
|
||
Later requests for the same document are served from the cache, revalidated after 24 h with a
|
||
conditional GET. `question` digests the document's text, page markers included, unless the
|
||
document is too long.
|
||
|
||
## Files
|
||
|
||
- Copy: `internal/api/pdf_test.go`
|
||
- Create: `internal/api/pdf.go` — everything PDF lives here
|
||
- Modify: `internal/api/api.go` — **only** the hooks listed in rule 7. `api.go` is already 840
|
||
lines; do not move code into it.
|
||
- Modify: `docs/implementer-log.md`
|
||
|
||
## Interfaces
|
||
|
||
```go
|
||
package api
|
||
|
||
const (
|
||
DefaultPDFTTL = 24 * time.Hour
|
||
DefaultPDFMaxQuestionChunks = 200
|
||
)
|
||
|
||
// in Server:
|
||
PDFRunner *pdftext.Runner // nil = &pdftext.Runner{} (the child is /proc/self/exe pdftext)
|
||
PDFPool *pdfpool.Pool // nil = pdfpool.New(2, 4, 20*time.Second)
|
||
PDFTTL time.Duration // 0 = DefaultPDFTTL
|
||
PDFMaxQuestionChunks int // 0 = DefaultPDFMaxQuestionChunks
|
||
|
||
// in v1Req:
|
||
Pages string `json:"pages"`
|
||
Grep string `json:"grep"`
|
||
```
|
||
|
||
Defaults are applied where the field is read: a zero `Server` from `New` must work. Build the
|
||
default pool once, with a `sync.Once` — never one per request.
|
||
|
||
## Rules
|
||
|
||
1. **Cache first.**
|
||
- Before fetching, look up `dcache.PDFKey(rawURL, s.callerClass(c))` in `s.cache()` (which
|
||
may be nil: then there is no cache).
|
||
- **Fresh** (now − `FetchedAt` < TTL): build the `*pdftext.Doc` from the entry (unmarshal
|
||
`Meta`; `Text` = `Pages`). Answer with no network call and no child. The source is marked
|
||
`"cached": true`.
|
||
- **Stale:** `s.Fetch.FetchIf(ctx, rawURL, e.ETag, e.LastModified)`.
|
||
- 304 → `TouchPDF(key, now)` and answer from the entry, with no child.
|
||
- 200 → go on as for a new document.
|
||
- **Not cached:** the usual `s.Fetch.Fetch`, under the existing `acquire`/`release`.
|
||
2. **A PDF** is `clean.Kind(res.ContentType, res.Body) == "pdf"`. Every other kind goes through
|
||
today's path unchanged. Use `clean.Kind` in place of the `MediaKind`/`Sniff` pair in both
|
||
`doFetch` and `cleanFetched`. `"other"` stays `415`.
|
||
3. **Extraction.**
|
||
- Call `s.PDFPool.Do(ctx, class+"|"+res.SHA256(), c.Name, fn)`, where `fn` runs
|
||
`s.PDFRunner.Run(jobCtx, res.Body)`.
|
||
- The key is the **content hash**, so identical bytes are extracted once however many callers
|
||
or URLs ask. The key includes the caller class, so a restricted caller never shares a job
|
||
with the fleet.
|
||
- Then `PutPDF` under the URL key with: the content hash; the response's `ETag` and
|
||
`Last-Modified`; `FetchedAt` = now; `Meta` = the JSON of `doc.Meta`; `Pages` = `doc.Text`.
|
||
4. **Errors**, all through the existing `writeErr` shape (`{"success":false,"error":…,"hint":…}`):
|
||
|
||
| cause | status | error | hint |
|
||
|---|---|---|---|
|
||
| `pdftext.ErrNoText` | 422 | `no text layer (scanned PDF?)` | `ocr` |
|
||
| `pdftext.ErrEncrypted` | 422 | `encrypted PDF (password required)` | |
|
||
| `pdftext.ErrUnreadable` | 422 | `unreadable PDF` | |
|
||
| `pdftext.ErrTimeout` | 422 | `pdf extraction timed out` | |
|
||
| `pdftext.ErrTooMuchText` | 422 | `pdf text over the output limit` | |
|
||
| `pdfpool.ErrBusy` | 503 | `pdf workers busy`, **header `Retry-After: 10`** | |
|
||
| `pdfpool.ErrCallerBusy` | 429 | `busy` | |
|
||
| `pdfdoc.ErrBadPages` / `ErrBadGrep` | 400 | the error's text | |
|
||
| `pages` or `grep` on a non-PDF | 400 | `pages and grep apply to PDFs only` | |
|
||
| `question` on a document over `PDFMaxQuestionChunks` chunks | 422 | `document too long for question (N chunks); use grep or pages` | |
|
||
|
||
A PDF path never answers 500 for anything the document did.
|
||
5. **`/v1/fetch`.**
|
||
- **Mode `clean`:** `content` is `pdfdoc.Render(doc, {Pages, Grep, OutCap: s.OutCap}).Content`.
|
||
- The envelope gains:
|
||
```
|
||
"pdf": {"pages", "title", "outline", "extracted", "first", "last", "map", "grep_hits", "note"}
|
||
```
|
||
`extracted` is `len(doc.Text)`; `outline` is `doc.Outline`; the rest come from `Rendered`.
|
||
- **Mode `digest`:** the text is `pdfdoc.Full(doc)`. First count
|
||
`len(digest.Split(full, digest.ChunkTokens))`; if it is over `PDFMaxQuestionChunks`, answer
|
||
the 422 above **before** any model call. Otherwise run the digest as for any page, with
|
||
`full` as the content.
|
||
- **Mode `raw`** on a PDF: 400 `raw mode does not apply to PDFs`.
|
||
- `sizes.raw_tokens` is the rune count of `Full(doc)` / 4 for a PDF (there is no body on a
|
||
cache hit).
|
||
6. **`/v2/scrape`.**
|
||
- `markdown` is `Render(doc, {OutCap: s.OutCap}).Content`; `html` is `""`.
|
||
- `metadata.title` is `doc.Title`; `metadata.numPages` is `doc.Pages`; `metadata.contentType`
|
||
is the response's (`application/pdf` on a cache hit).
|
||
7. **`api.go` hooks (the only edits allowed there):**
|
||
- the four `Server` fields and two `v1Req` fields;
|
||
- the `Kind` switch in `doFetch`/`cleanFetched`, now returning the doc;
|
||
- one call into `pdf.go` in `v1fetch` and in `scrape`;
|
||
- `Retry-After` in `writeFetchErr` for a 503.
|
||
|
||
`doFetch` and `cleanFetched` may gain a result, `doc *pdftext.Doc` (nil unless a PDF).
|
||
8. **Logging:**
|
||
- Keep today's `fetch`/`scrape` log lines and add `pdf_pages` (a count) to them.
|
||
- Never log the title, the outline or any text.
|
||
- A restricted caller's lines go through the existing helpers (host blanked); the given test
|
||
reads every log record.
|
||
|
||
## Facts
|
||
|
||
- The tests set `s.Digest = &digest.Engine{Cache: db}` only to give `s.cache()` a database: no
|
||
model is configured.
|
||
- The child in the tests is the test binary (`TestMain` in the given file).
|
||
- `setupAPI`, `postJSON` and `recHandler` come from `api_test.go` in the same package.
|
||
- `requestDeadline` (210 s) and `writeTimeout` (240 s) were raised by the owner for this path:
|
||
download ≤ 120 s + queue ≤ 20 s + extraction ≤ 60 s.
|
||
|
||
## Steps
|
||
|
||
- [ ] **1.** `git switch p5a`; copy the given test. **2. See it fail** (undefined fields).
|
||
- [ ] **3.** `pdf.go` (cache, extraction, errors, render), then the `api.go` hooks.
|
||
- [ ] **4.** `gofmt -w`; `go test -race -count=3 ./internal/api/` → `ok`: the new tests and
|
||
every existing api test (search, scrape, restricted logging, write timeout).
|
||
- [ ] **5.** `make gate`. **6.** Row `p5a/06-api`; commit.
|
||
|
||
```sh
|
||
git add internal/api docs/implementer-log.md
|
||
git commit -F .state/commit-msg.txt
|
||
```
|
||
|
||
## Done when
|
||
|
||
- `go test -race -count=3 ./internal/api/` is ok; `make gate` ok; given file byte-identical;
|
||
`api.go` grew by the hooks only (the owner will diff it).
|
||
|
||
## Stop and report if
|
||
|
||
- A rule here contradicts an existing api test.
|
||
- The pool's default wait (20 s) makes a test take longer than the test allows. The given tests
|
||
set their own pool where timing matters.
|