p5a: split task 06 (too large: looped 3h) into 06-api-pdf and 07-api-cache-pages; old 07 is 08
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5.5
@@ -0,0 +1,163 @@
|
||||
# P5a task 06: the PDF path through `/v1/fetch` and `/v2/scrape`
|
||||
|
||||
**Branch:** `p5a` (`git switch p5a`; `git status --short` must be empty, otherwise stop)
|
||||
**Commit subject:** `api: serve PDFs — extraction in the child under the pool, full text or map, errors`
|
||||
|
||||
> This task is half of the original task 06, which proved too large for one session. Task 07
|
||||
> adds the cache, `pages`, `grep` and the question limit. Do only what is here.
|
||||
|
||||
## Goal
|
||||
|
||||
A PDF URL fetched through `/v1/fetch` (mode `clean`) or `/v2/scrape` is extracted in the child
|
||||
process under the pool and answered with the whole text (page markers) or the map. Its
|
||||
failures get the errors below. **No cache in this task** (every request fetches and extracts);
|
||||
**no `pages` or `grep`**.
|
||||
|
||||
## Files
|
||||
|
||||
- Copy: `internal/api/pdf_test.go` (the whole given file; 7 of its 10 tests must pass after
|
||||
this task, listed in Done when)
|
||||
- Create: `internal/api/pdf.go`: every function named below lives here
|
||||
- Modify: `internal/api/api.go`, only the hooks in rule 6; `internal/fetch/fetch.go`, only
|
||||
the `RetryAfter` field in rule 5; `docs/implementer-log.md`
|
||||
|
||||
## What already exists (do not re-read these packages; this is their API)
|
||||
|
||||
```go
|
||||
// internal/clean
|
||||
func Kind(contentType string, body []byte) string // "html" | "json" | "text" | "pdf" | "other"
|
||||
|
||||
// internal/pdftext
|
||||
type OutlineItem struct{ Title string; Page, Level int }
|
||||
type Meta struct{ Pages int; Title string; Outline []OutlineItem }
|
||||
type Doc struct{ Meta; Text []string; Failed []int } // Text[i] = page i+1
|
||||
var ErrEncrypted, ErrUnreadable, ErrNoText, ErrTimeout, ErrTooMuchText error
|
||||
type Runner struct{ Exe string; Args, Env []string; Deadline time.Duration; MaxText int64; MaxPages int }
|
||||
func (r *Runner) Run(ctx context.Context, pdf []byte) (*Doc, error) // a zero Runner runs /proc/self/exe pdftext
|
||||
|
||||
// internal/pdfpool
|
||||
var ErrBusy, ErrCallerBusy error
|
||||
func New(slots, queue int, wait time.Duration) *Pool
|
||||
func (p *Pool) Do(ctx context.Context, key, caller string, fn func(context.Context) (any, error)) (any, error)
|
||||
|
||||
// internal/pdfdoc
|
||||
type Options struct{ Pages, Grep string; OutCap int }
|
||||
type Rendered struct{ Content string; Map bool; First, Last, GrepHits int; Note string }
|
||||
func Render(doc *pdftext.Doc, opt Options) (Rendered, error)
|
||||
func Full(doc *pdftext.Doc) string
|
||||
```
|
||||
|
||||
## Interfaces added here
|
||||
|
||||
```go
|
||||
package api
|
||||
|
||||
const (
|
||||
DefaultPDFTTL = 24 * time.Hour // used by task 07; declare it now
|
||||
DefaultPDFMaxQuestionChunks = 200 // used by task 07; declare it now
|
||||
)
|
||||
|
||||
// in Server (after Prom):
|
||||
PDFRunner *pdftext.Runner // nil = &pdftext.Runner{}
|
||||
PDFPool *pdfpool.Pool // nil = pdfpool.New(2, 4, 20*time.Second), built once (sync.Once)
|
||||
PDFTTL time.Duration // task 07
|
||||
PDFMaxQuestionChunks int // task 07
|
||||
|
||||
// in v1Req:
|
||||
Pages string `json:"pages"` // task 07
|
||||
Grep string `json:"grep"` // task 07
|
||||
```
|
||||
|
||||
## Rules
|
||||
|
||||
1. **`doFetch` and `cleanFetched` gain a fourth result, `doc *pdftext.Doc`,** which is nil unless
|
||||
the resource is a PDF. Each uses `clean.Kind(res.ContentType, res.Body)` in place of today's
|
||||
`MediaKind`/`Sniff` pair.
|
||||
- `"other"` stays the 415 exactly as today.
|
||||
- `"pdf"` calls `s.extractPDF(ctx, c, res)`; on success it returns
|
||||
`cl = &clean.Result{Kind: "pdf", Title: doc.Title}` and the doc.
|
||||
- `cleanFetched` has no caller at hand for the pool. For a PDF it returns the 415 as before:
|
||||
the recover path (archive snapshots) does not serve PDFs.
|
||||
2. **`func (s *Server) extractPDF(ctx context.Context, c *auth.Caller, res *fetch.Resource) (*pdftext.Doc, error)`**
|
||||
- Calls `s.pool().Do(ctx, s.callerClass(c)+"|"+res.SHA256(), c.Name, fn)`, where `fn` returns
|
||||
`s.runner().Run(jobCtx, res.Body)`.
|
||||
- `pool()` and `runner()` apply the defaults.
|
||||
- Type-assert the result with the two-value form.
|
||||
3. **`func pdfError(err error) error`** maps every PDF-path error to a `*fetch.Error`:
|
||||
|
||||
| cause | Status | Msg | Hint | RetryAfter |
|
||||
|---|---|---|---|---|
|
||||
| `pdftext.ErrNoText` | 422 | `no text layer (scanned PDF?)` | `ocr` | |
|
||||
| `pdftext.ErrEncrypted` | 422 | `encrypted PDF (password required)` | | |
|
||||
| `pdftext.ErrUnreadable` (also when wrapped) | 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` | | 10 |
|
||||
| `pdfpool.ErrCallerBusy` | 429 | `busy` | | |
|
||||
| anything else (a panic recovered by the pool) | 422 | `unreadable PDF` | | |
|
||||
|
||||
Use `errors.Is`. A PDF never produces a 500.
|
||||
4. **Rendering** (`pdf.go`), `func (s *Server) renderPDF(doc *pdftext.Doc) (pdfdoc.Rendered, error)`:
|
||||
`pdfdoc.Render(doc, pdfdoc.Options{OutCap: s.OutCap})`.
|
||||
- **`/v1/fetch`**, mode `clean` with a doc:
|
||||
- `content` is `Rendered.Content`;
|
||||
- the envelope gains
|
||||
`"pdf": {"pages": doc.Pages, "title": doc.Title, "outline": doc.Outline, "extracted": len(doc.Text), "first", "last", "map", "grep_hits", "note"}`,
|
||||
the last five from `Rendered`;
|
||||
- `sizes.raw_tokens` is the rune count of `pdfdoc.Full(doc)` / 4.
|
||||
- Mode `raw` with a doc: `400 raw mode does not apply to PDFs`.
|
||||
- Mode `digest` with a doc: run the digest exactly as today but with `pdfdoc.Full(doc)` as the
|
||||
content. Task 07 adds the length limit before it.
|
||||
- **`/v2/scrape`** with a doc:
|
||||
- `markdown` is `Rendered.Content`; `html` is `""`;
|
||||
- `metadata.title` is `doc.Title`;
|
||||
- `metadata.numPages` is `doc.Pages`;
|
||||
- `metadata.contentType` is the response's.
|
||||
5. **`internal/fetch/fetch.go`:** add `RetryAfter int` to `fetch.Error` (seconds, 0 = none).
|
||||
`writeFetchErr` sets the `Retry-After` header when it is non-zero, before `writeErr`. This is
|
||||
the only change allowed in the fetch package.
|
||||
6. **`api.go` hooks, and nothing else there:**
|
||||
- the `Server` and `v1Req` fields;
|
||||
- the two `Kind` switches (rule 1);
|
||||
- the new result at the three call sites of `doFetch`/`cleanFetched`;
|
||||
- one `if doc != nil { … }` block in `v1fetch` and in `scrape` that calls into `pdf.go`;
|
||||
- the `Retry-After` line in `writeFetchErr`;
|
||||
- `pdf_pages` (a count) added to the existing `fetch` and `scrape` log lines.
|
||||
|
||||
Never log a title, an outline or any text. Restricted callers go through the existing helpers.
|
||||
|
||||
## Facts
|
||||
|
||||
- The given test's child is the test binary itself (its `TestMain` turns into `pdftext` when
|
||||
`SIFT_PDFTEXT_CHILD=1`). The rig builds `s.PDFRunner` with that environment; you never start
|
||||
a child yourself outside `Runner.Run`.
|
||||
- `setupAPI`, `postJSON` and `recHandler` come from `api_test.go` in the same package.
|
||||
- `s.cache()` may be nil; this task never uses it.
|
||||
|
||||
## Steps
|
||||
|
||||
- [ ] **1.** `git switch p5a`; copy the given test.
|
||||
- [ ] **2. See it fail:** `go test -count=1 ./internal/api/` fails to compile (undefined fields).
|
||||
- [ ] **3.** `fetch.Error.RetryAfter`. **4.** `pdf.go`. **5.** The `api.go` hooks.
|
||||
- [ ] **6.** `gofmt -w`; then
|
||||
`go test -race -count=3 -run 'TestPDFCleanFetch|TestPDFMap|TestPDFErrors|TestPDFTimeout|TestPDFBusy|TestPDFRestrictedCaller|TestPDFScrape' ./internal/api/` → `ok`,
|
||||
and `go test -race -count=1 ./internal/api/ -run 'Test[^P]'` (every existing api test) → `ok`.
|
||||
- [ ] **7.** Row `p5a/06-api-pdf`. Its Gate column says `pass (7 of 10 PDF tests; 3 are task 07's)`.
|
||||
Commit.
|
||||
|
||||
```sh
|
||||
git add internal/api internal/fetch/fetch.go docs/implementer-log.md
|
||||
git commit -F .state/commit-msg.txt
|
||||
```
|
||||
|
||||
## Done when
|
||||
|
||||
- The seven named tests pass under `-race -count=3`, and every non-PDF api test passes.
|
||||
- The three remaining given tests fail only on `pages`, `grep`, the cache or the question
|
||||
limit: `TestPDFPagesAndGrep`, `TestPDFCache`, `TestPDFQuestionTooLong`. That is task 07.
|
||||
`make gate` is expected red on exactly those three and nothing else. Say so in the log row.
|
||||
- The given file is byte-identical.
|
||||
|
||||
## Stop and report if
|
||||
|
||||
- Any of the seven fails for a reason that needs the cache, `pages` or `grep`.
|
||||
Reference in New Issue
Block a user