77 lines
5.4 KiB
Markdown
77 lines
5.4 KiB
Markdown
# P5a implementation plan: PDF documents
|
||
|
||
> **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:** `docs/P5a.md`. sift fetches, extracts and serves PDFs, so agents stop running
|
||
`pdftotext` in the shell. Engine: `github.com/go-pdfkit/extract` v0.3.0 (bake-off, P5a §3),
|
||
vendored by the owner before the run.
|
||
|
||
| Task | Package(s) | Given |
|
||
|---|---|---|
|
||
| 01-kind-fetch | `internal/clean`, `internal/fetch` | `clean/kind_test.go`, `fetch/pdf_test.go` |
|
||
| 02-pdftext | `internal/pdftext`, `cmd/sift` (one dispatch line) | `pdftext/pdftext_test.go`, `internal/pdftest/pdftest.go`, `tests/testdata/pdf/*` |
|
||
| 03-dcache | `internal/dcache` | `dcache/pdf_test.go` |
|
||
| 04-pdfpool | `internal/pdfpool` | `pdfpool/pdfpool_test.go` |
|
||
| 05-pdfdoc | `internal/pdfdoc` | `pdfdoc/pdfdoc_test.go` |
|
||
| 06-api-pdf | `internal/api` (+ one field in `internal/fetch`) | `api/pdf_test.go` |
|
||
| 07-api-cache-pages | `internal/api` | (same file; its last 3 tests) |
|
||
| 08-digest-cli-docs | `internal/digest`, `internal/metrics`, `cmd/sift`, `internal/api` (metric calls), `skill/`, `nix/module.nix`, `README.md` | `digest/pages_test.go`, `metrics/pdf_test.go` |
|
||
|
||
**Order matters:** 05 imports 02's types; 06 and 07 use 01–05; 08 wires metrics into 06/07's file.
|
||
|
||
**How this plan was made:** acceptance tests first, no reference implementation. The given tests
|
||
compiled against a panic-only skeleton of every new name, on this repository at the plan
|
||
commit, and failed there for the intended reasons; every earlier test stayed green. The PDFs the
|
||
tests use are built by `internal/pdftest` (a given file) at test time. The only binary fixtures
|
||
are two small qpdf-encrypted copies of the three-page test document (`tests/testdata/pdf/README.md`).
|
||
|
||
**Owner changes made before the run** (plan commit):
|
||
- `go.mod`, `go.sum`, `vendor/`: `go-pdfkit/extract` v0.3.0, `go-pdfkit/reader` v0.6.0 (and its
|
||
`pdffont`), pinned through `tools.go` until task 02 imports them. `flake.nix` uses
|
||
`vendorHash = null`, so the nix build reads `vendor/`.
|
||
- `internal/api/api_test.go`: `TestUnsupportedType` now uses `application/zip`; a PDF is no
|
||
longer unsupported.
|
||
- `gofmt -w` on `internal/clean/clean.go`, `internal/guard/guard_test.go`,
|
||
`internal/guard/iana.go` (whitespace only), so the new `make gate` target passes on master.
|
||
- `AGENTS.md`, `tools/run-plan.sh`, `docs/implementer-log.md`, `make gate`.
|
||
|
||
**Attribution:** `tools/run-plan.sh` gives the model the exact trailer
|
||
`Co-Authored-By: <model> <<model>@llama.cpp.invalid>` and refuses a commit without it.
|
||
|
||
**Where it runs:** a separate checkout (`~/src/sift-impl` on straylight), branch `p5a`. Nothing
|
||
reaches orion until the owner's review, the merge, and straylight's pin bump.
|
||
|
||
## Changes during the run
|
||
|
||
- 2026-09-25, task 01: **owner test fault.** `TestFetchIf` took its mutex with `defer mu.Unlock()`
|
||
and then made a third request, whose server handler needs that mutex: a deadlock that surfaced as
|
||
a 20 s upstream timeout. Ornith spent 55 minutes debugging it correctly (its notes had the
|
||
handler blocked on `mu.Lock` at the test line) and ended on a refused `/tmp` read, with the
|
||
implementation done but debug prints in `pdf.go` and a `zz_debug_test.go`. Owner fixed the test
|
||
(copy the slice under the lock, release before the next request), removed the debug scaffolding,
|
||
and resumed the session to finish and commit.
|
||
- 2026-09-26, task 06 (the original, API integration in one task): **owner sizing fault.** The
|
||
session looped for 3 h 13 min (731 KB log, repeated context compaction, false beliefs about
|
||
the tree: "the given test is committed", "the child is /bin/true") without a compiling
|
||
`pdf.go`. It is the same failure as crossbar v1/06: one task spanning detection, pool,
|
||
errors, rendering, cache, pages, grep, digest and scrape. Killed by pid. Split into
|
||
**06-api-pdf** (detection, extraction under the pool, errors, full text/map, scrape; 7 of the
|
||
10 given tests) and **07-api-cache-pages** (cache and revalidation, pages, grep, the question
|
||
limit; the other 3). The old 07 is now 08. Both task texts state the exact API of tasks 01–05,
|
||
so the model need not re-read five packages. The working tree was reset: the draft
|
||
`pdf.go`, the `api.go` hooks and a `fetch.Error.RetryAfter` field that was outside the task's
|
||
files (task 06 now allows exactly that field).
|
||
- 2026-09-26, the given test's gate note: task 06 ends with `make gate` red on exactly the three
|
||
task-07 tests (the pattern crossbar v2.2 used); the gate runs green after 07.
|
||
- 2026-09-26, tasks 06–08 after the split: 52, 19 and 14 min, each first-gate. `run-plan`: all
|
||
tasks done at 04:37.
|
||
- **Owner review** (details in `docs/implementer-log.md`): end-to-end on real PDFs (arXiv, NIST,
|
||
RP2040 datasheet, a PhD thesis) against dixie's 9B found one real defect the given tests could
|
||
not see — **no outline for any LaTeX or Acrobat PDF**: named destinations were skipped (my task
|
||
02 said to skip them) and reader's `Dict.Get` answers `reader.Null{}`, not nil, for a missing
|
||
key, which hid the `/A` action. Fixed with a name-tree lookup; `internal/pdftest` gained
|
||
`NamedDests` (owner change to a given file; the copy here is the new reference). Also fixed: a
|
||
race on the default pool, revalidation outside the per-host limit (my spec), the runner waiting
|
||
for the deadline on over-limit output, `Accept` lacking `application/pdf` (my spec).
|