P5a plan: PDF documents — seven Ornith tasks with acceptance tests first
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>
This commit is contained in:
Claude Opus 5.5
@@ -0,0 +1,108 @@
|
||||
# P5a task 07: page citations, metrics, CLI flags, skill, unit, README
|
||||
|
||||
**Branch:** `p5a` (`git switch p5a`; `git status --short` must be empty, otherwise stop)
|
||||
**Commit subject:** `P5a: digests cite pages; sift_pdf_* metrics; sift fetch --pages/--grep; skill, unit and README`
|
||||
|
||||
## Goal
|
||||
|
||||
Finish P5a:
|
||||
- digests of PDFs cite `(p. N)`;
|
||||
- the PDF path is visible in `/metrics`;
|
||||
- `sift fetch` can ask for pages and grep;
|
||||
- the agents' skill tells them PDFs work;
|
||||
- the unit gets the memory the child needs;
|
||||
- the README describes all of it.
|
||||
|
||||
## Files
|
||||
|
||||
- Copy: `internal/digest/pages_test.go`, `internal/metrics/pdf_test.go`, `cmd/sift/fetchpdf_test.go`
|
||||
- Modify:
|
||||
- `internal/digest/digest.go` (prompt line, `PromptVersion`)
|
||||
- `internal/metrics/metrics.go`
|
||||
- `internal/api/pdf.go` (the metric calls)
|
||||
- `cmd/sift/fetchcmd.go`
|
||||
- `skill/SKILL.md`
|
||||
- `nix/module.nix` (**only** `MemoryMax`)
|
||||
- `README.md`
|
||||
- `docs/implementer-log.md`
|
||||
|
||||
## Rules
|
||||
|
||||
1. **Digest.**
|
||||
- Add one sentence to `systemPrompt`: `When the document has "--- page N ---" markers, cite
|
||||
the page of each fact as (p. N).`
|
||||
- Set `PromptVersion = "3"`. The digest cache keys on it, so digests made with the old
|
||||
prompt are not served.
|
||||
- `Split` already keeps markers whole; the given test pins that.
|
||||
2. **Metrics** (`metrics.go`):
|
||||
- `func (s *Set) ObservePDF(outcome string, d time.Duration, pages int)`. It feeds:
|
||||
- the counter `sift_pdf_extract_total{outcome}`;
|
||||
- the histogram `sift_pdf_extract_seconds`;
|
||||
- the counter `sift_pdf_pages_total`.
|
||||
|
||||
`outcome` must be one of `ok`, `no_text`, `encrypted`, `unreadable`, `timeout`,
|
||||
`too_large`, `busy`; anything else is counted as `other`. The label never carries raw
|
||||
text: the given test sends `blocked: evil.example`.
|
||||
- `func (s *Set) ObservePDFWait(d time.Duration)` feeds the histogram
|
||||
`sift_pdf_pool_wait_seconds`.
|
||||
- `func (s *Set) PDFCacheHit()` feeds the counter `sift_pdf_cache_hits_total`.
|
||||
- Register them on the Set's own registry, like `sift_requests_total`.
|
||||
- In `internal/api/pdf.go`:
|
||||
- call `ObservePDF` once per extraction attempt, with the outcome and the pages extracted;
|
||||
- call `ObservePDFWait` with the time spent waiting in `PDFPool.Do` before the job
|
||||
started (measure around `Do`: total minus the job's own time);
|
||||
- call `PDFCacheHit` for every answer served from the cache, fresh or after a 304.
|
||||
3. **CLI** (`fetchcmd.go`).
|
||||
- Parsing becomes a struct, and today's tuple function stays as a wrapper so the existing
|
||||
tests keep compiling:
|
||||
```go
|
||||
type fetchOpts struct {
|
||||
URL, Mode, JQ, Question, Pages, Grep string
|
||||
NoContext, Recover, JSON bool
|
||||
}
|
||||
func parseFetchOpts(args []string) (fetchOpts, error) // the parser; knows --pages and --grep (both take a value)
|
||||
func parseFetchArgs(args []string) (…the same 8 results as today…) // calls parseFetchOpts
|
||||
func fetchBody(o fetchOpts) map[string]any // the /v1/fetch request body
|
||||
```
|
||||
- `fetchBody` builds today's body: url, mode (defaulting to digest when there is a question,
|
||||
else clean), jq, question, context (question set and not `--no-context`), recover. It adds
|
||||
`"pages"` and `"grep"` **only when set**.
|
||||
- The command uses `parseFetchOpts` and `fetchBody`.
|
||||
- Add `[--pages a-b] [--grep RE]` to `fetchUsage`.
|
||||
- The given `cmd/sift/fetchpdf_test.go` and the existing flag tests must pass.
|
||||
4. **Skill** (`skill/SKILL.md`):
|
||||
- Remove the `415 unsupported type` row's mention of PDF.
|
||||
- Add a short "PDFs" section:
|
||||
- PDFs work;
|
||||
- a long one answers with a map (outline with page numbers, first pages);
|
||||
- then use `--pages 41-60` or `--grep 'pattern'`, or `--question` for a digest that cites
|
||||
pages;
|
||||
- scanned PDFs return `no text layer`;
|
||||
- never download a PDF and run `pdftotext` yourself.
|
||||
5. **Unit** (`nix/module.nix`): `MemoryMax = "1G";` (was `"512M"`). Nothing else in the file
|
||||
changes.
|
||||
6. **README:**
|
||||
- a "PDF documents" section covering: detection, limits (100 MB, 120 s download, 60 s
|
||||
extraction, 2,000 pages), the child process, the pool (2 slots, 4 waiting, 20 s), the
|
||||
cache (24 h, conditional GET), the answers (full / map / pages / grep / digest with
|
||||
citations), the errors table from task 06, and the metrics;
|
||||
- note that the engine is `go-pdfkit/extract` v0.3.0, chosen by the bake-off in `docs/P5a.md` §3.
|
||||
|
||||
## Steps
|
||||
|
||||
- [ ] **1.** `git switch p5a`; copy the three given tests. **2. See them fail.**
|
||||
- [ ] **3.** digest, **4.** metrics and the calls in `pdf.go`, **5.** CLI, **6.** skill, unit,
|
||||
README.
|
||||
- [ ] **7.** `gofmt -w`; `make gate`.
|
||||
- [ ] **8.** `make build && bin/sift fetch 2>&1 | grep -c -- '--pages'` prints `1`.
|
||||
- [ ] **9.** Row `p5a/07-digest-cli-docs`; commit.
|
||||
|
||||
```sh
|
||||
git add internal/digest internal/metrics internal/api/pdf.go cmd/sift skill nix/module.nix README.md docs/implementer-log.md
|
||||
git commit -F .state/commit-msg.txt
|
||||
```
|
||||
|
||||
## Done when
|
||||
|
||||
- All tests pass; `make gate` ok; given files byte-identical; the `nix/module.nix` diff is the
|
||||
one line.
|
||||
Reference in New Issue
Block a user