P5a review: outlines from named destinations; pool race; fast over-limit; revalidation under the limiter; Accept pdf

- pdftext/outline: resolve named destinations through the catalog /Dests and the /Names /Dests tree, resolve indirect /Dest, /A and /Title, and treat reader.Null (what Dict.Get returns for a missing key) as absent. Before this every LaTeX and Acrobat PDF had an empty outline. pdftest gains NamedDests; TestOutlineNamedDestinations. - api: pool() always goes through its Once (data race on concurrent first use); TestDefaultPoolConcurrentFirstUse. Revalidation of a stale cache entry counts against the caller/host limit. - pdftext/runner: output over MaxText kills the child at once instead of waiting for the deadline; TestRunnerTooMuchTextFailsFast. - fetch: Accept includes application/pdf;q=0.8. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-26 04:48:17 -07:00
parent 7e78de633f
commit aafc1d1c28
10 changed files with 287 additions and 65 deletions
@@ -12,3 +12,29 @@ One row per task session that ends in a commit. Newest last.
 | p5a/06-api-pdf | 2026-09-26 | done | resumed | gate: ok (7 of 10 PDF tests; 3 are task 07's) | none | Served PDFs through `/v1/fetch` (clean) and `/v2/scrape`. Added `RetryAfter int` to `fetch.Error` and a `Retry-After` header line guarded by `> 0`. Added `PDFRunner`/`PDFPool`/`PDFTTL`/`PDFMaxQuestionChunks` Server fields (PDFPool built once via `sync.Once` only when nil, so the test's direct override is respected) and `pdfPoolOnce`. Routed `clean.Kind` in `doFetch`/`cleanFetched` (html/json/text switched, "other"→415, "pdf"→extractPDF); `doFetch`'s switch routes "text" explicitly so a PDF never runs `clean.Text` first. New `internal/api/pdf.go`: `DefaultPDFTTL`/`DefaultPDFMaxQuestionChunks` (task 07), `runner()`/`pool()` defaults, `extractPDF` (pool key `callerClass|SHA256`, two-value assert, `fmt.Errorf("pdf: bad result %T")`), `pdfError` (422 for the five pdftext errors + panic default, 503+RetryAfter 10 for ErrBusy, 429 for ErrCallerBusy), `renderPDF` (`pdfdoc.Render`, whole text or map). `doFetch` returns the 4th `doc *pdftext.Doc`; digest uses `pdfdoc.Full(doc)` (computed once) as content, clean uses `renderPDF.Content`, envelope `pdf` object has pages/title/outline/extracted/first/last/map/grep_hits/note, raw mode→400. Scrape markdown=`renderPDF.Content`, html="", `metadata.numPages`=doc.Pages. 7 of 10 given tests pass under -race -count=3; the 3 failures (`TestPDFPagesAndGrep`, `TestPDFCache`, `TestPDFQuestionTooLong`) are task 07 tests that reference fields not in this task, exactly as the task doc (lines 145, 157-158) predicts; nothing else fails. Given files byte-identical. |
 | p5a/08-digest-cli-docs | 2026-09-26 | done | 1 | gate: ok | none | Added the page-cite sentence to the digest `systemPrompt` ("When the document has `--- page N ---` markers, cite the page of each fact as (p. N).") and set `PromptVersion = "3"`. Added the `sift_pdf_extract_total{outcome}`, `sift_pdf_extract_seconds`, `sift_pdf_pages_total`, `sift_pdf_pool_wait_seconds`, `sift_pdf_cache_hits_total` series to `internal/metrics` (`pdf.go` package) and wired `ObservePDF`/`ObservePDFWait`/`PDFCacheHit` into `internal/api/pdf.go` (`ObservePDF` once per call after `extractPDF`, outcome classified by `pdfOutcome` so raw text never becomes a label; `ObservePDFWait` only when the caller actually waited on `pool().Do`; `PDFCacheHit` on fresh hits and 304s). Refactored `cmd/sift/fetchcmd.go` into a `fetchOpts` struct with `parseFetchOpts` (returns `fetchOpts`, not `string, error`) and a `fetchBody(o fetchOpts) map[string]any` that does the digest/clean default and adds `pages`/`grep` only when set; `--pages`/`--grep` added to `fetchValueFlags`. Updated `skill/SKILL.md` (PDFs section; removed PDF mention from the 415 row). Unit: `nix/module.nix` `MemoryMax = "1G"` (one line). README: added a "PDF documents" section covering detection, limits (100 MB/120 s download, 60 s extraction, 2,000 pages), the child process, the 2-slot/4-wait/20 s pool, the 24 h conditional-GET cache, the answers, the task-06 errors table, and the metrics; noted the `go-pdfkit/extract` v0.3.0 engine chosen by the `docs/P5a.md` §3 bake-off. All tests pass under -race, make gate ok, three given files byte-identical, nix diff is one line. |
 | p5a/07-api-cache-pages | 2026-09-26 | done | 1 | gate: ok | none | Implemented the PDF document cache with revalidation in `doFetch`: `pdfFromCache` (miss / fresh within `PDFTTL` / stale → `FetchIf` with 304→`TouchPDF`+answer-fresh / 200→return response with ok=false and no second GET) and `storePDF` after `extractPDF` (`ContentHash=res.SHA256`, ETag/Last-Modified from headers, FetchedAt=now, Meta=JSON of `doc.Meta`, Pages=`doc.Text`). `pages`/`grep` passed into `pdfdoc.Options` in `renderPDF`; `ErrBadPages`/`ErrBadGrep`→400 with the error's own text, non-PDF→400 "pages and grep apply to PDFs only" before the doc block. Question limit in digest mode: `n=len(digest.Split(pdfdoc.Full(doc), ChunkTokens))`, `n>PDFMaxQuestionChunks`→422 "document too long for question (n chunks); use grep or pages" before any digest call. Envelope `cached` set true for cache answers via the synthetic cache res's empty body (`len(res.Body)==0`); the one `doFetch` change beyond the rule-1 call was reusing that 200 response instead of a second `Fetch`. Added `fmt` import to api.go. Given file byte-identical; make gate ok. |
+
+## P5a review (owner, 2026-09-26)
+
+Checked: 8 task commits (`13365a2` … `7e78de6`), each with the `Co-Authored-By: ornith-1.5-35b-a3b`
+trailer; every given file byte-identical to its plan copy; `internal/guard`, `internal/auth`,
+`go.mod`, `go.sum`, `vendor/` untouched; `nix/module.nix` changed only in `MemoryMax`; `make gate`
+ok; `go test -race -count=3 ./...` ok.
+
+End-to-end with the real binary, the real guard, a real cache and dixie's 9B for digests, on
+public PDFs: Attention (15 pp, whole text, 1.9 s), GPT-3 (75 pp, map, 2.6 s), NIST SP 800-63B,
+the RP2040 datasheet (642 pp, 4.5 s), Karpathy's thesis (14.7 MB, 16 s including download).
+`pages` and `grep` from the cache in about 1.5 ms; a `question` digest with correct page
+citations (p. 1, 7, 8) in 11 s; `/v2/scrape` shape; `sift_pdf_*` metrics; no title or text in
+the logs.
+
+| # | Finding | Severity | Fault | Fix |
+|---|---|---|---|---|
+| 1 | **No outline for any LaTeX (hyperref) or Acrobat PDF** — every arXiv paper and the NIST PDF showed 0 items (qpdf: 22–120). Named destinations were skipped (task 02 said to skip them), `/Dest`/`/A`/titles were not resolved when indirect, and reader's `Dict.Get` returns `reader.Null{}` for a missing key, so the `/A` branch never ran. | high (feature absent on most papers) | task + model | owner: name-tree and `/Dests` lookup, `resolve` treats Null as absent, indirect titles; `pdftest` `NamedDests`; `TestOutlineNamedDestinations` (fails on the old code) |
+| 2 | `pool()` read `PDFPool` outside its `sync.Once` — a data race on concurrent first use with no pool configured (the production default). | medium | model | owner: always through the Once; `TestDefaultPoolConcurrentFirstUse` (race on the old code) |
+| 3 | Output over `MaxText` returned only at the deadline (the runner waited for stderr EOF while the child blocked on a full stdout pipe). | low | model | owner: check size first; `TestRunnerTooMuchTextFailsFast` (30 s on the old code) |
+| 4 | Stale-cache revalidation ran outside `acquire`, so it was not counted against the per-caller/host limit. | low | task (said "before acquire") | owner |
+| 5 | Fetch `Accept` lacked `application/pdf` (P5a §4). | low | task (not in task 01's rules) | owner |
+
+Process: 1 owner test deadlock (task 01, 55 min), 1 oversized task (06, looped 3 h, split),
+2 hangs from OpenCode 1.15 waiting on a piped stdin (driver fixed), 1 refusal-ending (task 01),
+1 malformed tool call (task 04). Every task that ran to the gate passed it first time.
@@ -314,7 +314,7 @@ type v1Req struct {
 	JQ       string `json:"jq"`
 	Recover  bool   `json:"recover"`

-	// Pages narrows a PDF digest to "2-3"; Grep matches lines in it. Optional; task-07.
+	// Pages ("41-60") and Grep (an RE2 pattern) narrow a PDF answer in clean mode; see pdfdoc.
 	Pages string `json:"pages"`
 	Grep  string `json:"grep"`
 }
@@ -35,11 +35,12 @@ func (s *Server) runner() *pdftext.Runner {

 // pool returns the extraction pool, built once and only when none is configured.
 func (s *Server) pool() *pdfpool.Pool {
-	if s.PDFPool == nil {
+	// Always through the Once: reading PDFPool outside it would race with the first writer.
 	s.pdfPoolOnce.Do(func() {
+		if s.PDFPool == nil {
 			s.PDFPool = pdfpool.New(2, 4, 20*time.Second)
-		})
 		}
+	})
 	return s.PDFPool
 }

@@ -158,7 +159,13 @@ func (s *Server) pdfFromCache(ctx context.Context, c *auth.Caller, rawURL string
 		res, doc := s.pdfFromEntry(rawURL, e)
 		return res, doc, true, nil
 	}
+	// A revalidation is an upstream request like any other: it counts against the caller's and
+	// the host's limits.
+	if err := s.acquire(c.Name, rawURL); err != nil {
+		return nil, nil, false, err
+	}
 	res, err := s.Fetch.FetchIf(ctx, rawURL, e.ETag, e.LastModified)
+	s.release(c.Name, rawURL)
 	if err != nil {
 		return nil, nil, false, err
 	}
@@ -0,0 +1,30 @@
+package api
+
+// Owner review (P5a): the default pool is built once, safely, when concurrent first requests
+// arrive on a server with no pool configured (run under -race).
+
+import (
+	"sync"
+	"testing"
+)
+
+func TestDefaultPoolConcurrentFirstUse(t *testing.T) {
+	s, _ := setupAPI(t, nil)
+	var wg sync.WaitGroup
+	got := make(chan any, 16)
+	for i := 0; i < 16; i++ {
+		wg.Add(1)
+		go func() { defer wg.Done(); got <- s.pool() }()
+	}
+	wg.Wait()
+	close(got)
+	var first any
+	for p := range got {
+		if first == nil {
+			first = p
+		}
+		if p != first || p == nil {
+			t.Fatal("pool() returned different or nil pools")
+		}
+	}
+}
@@ -101,7 +101,7 @@ func (f *Fetcher) doFetch(ctx context.Context, rawURL, etag, lastModified string
 		return nil, errf(400, "", "invalid url")
 	}
 	req.Header.Set("User-Agent", f.ua())
-	req.Header.Set("Accept", "text/html,application/xhtml+xml,application/json;q=0.9,text/plain;q=0.8,*/*;q=0.1")
+	req.Header.Set("Accept", "text/html,application/xhtml+xml,application/json;q=0.9,text/plain;q=0.8,application/pdf;q=0.8,*/*;q=0.1")
 	if etag != "" {
 		req.Header.Set("If-None-Match", etag)
 	}
@@ -25,6 +25,10 @@ type Doc struct {
 	Pages   []string
 	Outline []Outline
 	Scanned []int
+	// NamedDests writes the outline the way LaTeX's hyperref and Acrobat do: each item is a
+	// /GoTo action to a named destination (resolved through the catalog's /Names /Dests tree)
+	// and its /Title is an indirect object. Without it items carry a direct /Dest array.
+	NamedDests bool
 }

 // Build returns the PDF bytes for d. It panics on a Doc it cannot express (an outline entry
@@ -51,9 +55,15 @@ func Build(d Doc) []byte {
 		scanned[p] = true
 	}

+	// With NamedDests the outline items are followed by one title object per item and then
+	// the name tree, so the numbers below stay fixed by position.
+	nameTree := outlineRoot + 1 + 2*len(d.Outline)
 	catalog := "<< /Type /Catalog /Pages 2 0 R"
 	if len(d.Outline) > 0 {
 		catalog += fmt.Sprintf(" /Outlines %d 0 R /PageMode /UseOutlines", outlineRoot)
+		if d.NamedDests {
+			catalog += fmt.Sprintf(" /Names << /Dests %d 0 R >>", nameTree)
+		}
 	}
 	obj(catalog + " >>")
 	var kids []string
@@ -93,6 +103,9 @@ func Build(d Doc) []byte {
 			}
 			n := first + i
 			item := fmt.Sprintf("<< /Title (%s) /Parent %d 0 R /Dest [%d 0 R /XYZ null null null]", esc(o.Title), outlineRoot, pageObj(o.Page-1))
+			if d.NamedDests {
+				item = fmt.Sprintf("<< /Title %d 0 R /Parent %d 0 R /A << /S /GoTo /D (dest.%d) >>", last+1+i, outlineRoot, i)
+			}
 			if i > 0 {
 				item += fmt.Sprintf(" /Prev %d 0 R", n-1)
 			}
@@ -101,6 +114,14 @@ func Build(d Doc) []byte {
 			}
 			obj(item + " >>")
 		}
+		if d.NamedDests {
+			var names []string
+			for i, o := range d.Outline {
+				obj(fmt.Sprintf("(%s)", esc(o.Title)))
+				names = append(names, fmt.Sprintf("(dest.%d) [%d 0 R /XYZ null null null]", i, pageObj(o.Page-1)))
+			}
+			obj(fmt.Sprintf("<< /Names [%s] >>", strings.Join(names, " ")))
+		}
 	}

 	xref := b.Len()
@@ -24,27 +24,33 @@ func walkOutline(doc *reader.Document) []OutlineItem {
 	if !ok {
 		return out
 	}
-	outlines, err := doc.Resolve(catDict.Get("Outlines"))
-	if err != nil || outlines == nil {
-		return out
-	}
-	outlinesDict, ok := reader.ToDict(outlines)
+	outlines, ok := reader.ToDict(resolve(doc, catDict.Get("Outlines")))
 	if !ok {
 		return out
 	}
-	first, ok := toRef(outlinesDict.Get("First"))
+	first, ok := toRef(outlines.Get("First"))
 	if !ok {
 		return out
 	}
+	o := &outliner{doc: doc, cat: catDict}
 	seen := map[reader.Ref]bool{}
-	walkChildren(doc, first, 1, seen, &out)
+	o.walkChildren(first, 1, seen, &out)
 	return out
 }

+// outliner resolves destinations for one document. The page index and the named
+// destinations are built once, on first use.
+type outliner struct {
+	doc   *reader.Document
+	cat   reader.Dict
+	pages map[reader.Ref]int
+	names map[string]reader.Object // named destinations from /Dests and the /Names /Dests tree
+}
+
 // walkChildren walks the /Next chain from ref at the given level, emitting each
 // resolvable item and recursing into its /First children (level + 1). It stops
 // after maxOutlineItems items and never visits an object twice.
-func walkChildren(doc *reader.Document, ref reader.Ref, level int, seen map[reader.Ref]bool, out *[]OutlineItem) {
+func (o *outliner) walkChildren(ref reader.Ref, level int, seen map[reader.Ref]bool, out *[]OutlineItem) {
 	cur := ref
 	for cur != (reader.Ref{}) {
 		if seen[cur] {
@@ -52,24 +58,20 @@ func walkChildren(doc *reader.Document, ref reader.Ref, level int, seen map[read
 		}
 		seen[cur] = true

-		obj, err := doc.Resolve(cur)
-		if err != nil || obj == nil {
-			break
-		}
-		d, ok := reader.ToDict(obj)
+		d, ok := reader.ToDict(resolve(o.doc, cur))
 		if !ok {
 			break
 		}

-		if page, ok := destPage(doc, d); ok {
-			*out = append(*out, OutlineItem{Title: textString(d.Get("Title")), Page: page, Level: level})
+		if page, ok := o.destPage(d); ok {
+			*out = append(*out, OutlineItem{Title: textString(resolve(o.doc, d.Get("Title"))), Page: page, Level: level})
 			if len(*out) >= maxOutlineItems {
 				return
 			}
 		}

 		if child, ok := toRef(d.Get("First")); ok {
-			walkChildren(doc, child, level+1, seen, out)
+			o.walkChildren(child, level+1, seen, out)
 			if len(*out) >= maxOutlineItems {
 				return
 			}
@@ -83,57 +85,130 @@ func walkChildren(doc *reader.Document, ref reader.Ref, level int, seen map[read
 	}
 }

-// destPage resolves an item's destination to a 1-based page number. It reports
-// ok only when the destination is an array whose first element is a page
-// reference that maps to a real page; a named or otherwise unresolvable
-// destination is not ok, so the caller skips the item.
-func destPage(doc *reader.Document, d reader.Dict) (int, bool) {
-	if ref, ok := destRef(doc, d); ok {
-		if page, ok := pageOf(doc, ref); ok {
-			return page, true
-		}
-	}
-	return 0, false
-}
-
-// destRef finds the destination page reference, from /Dest or from a /GoTo
-// action's /D.
-func destRef(doc *reader.Document, d reader.Dict) (reader.Ref, bool) {
-	if o := d.Get("Dest"); o != nil {
-		if arr, ok := reader.ToArray(o); ok && len(arr) > 0 {
-			if ref, ok := arr[0].(reader.Ref); ok {
-				return ref, true
-			}
-		}
-	}
-	if o := d.Get("A"); o != nil {
-		if a, ok := reader.ToDict(o); ok {
+// destPage resolves an item's destination to a 1-based page number: /Dest, or a
+// /GoTo action's /D; either may be an explicit array, a dictionary holding /D,
+// or a name looked up in the catalog's named destinations (LaTeX's hyperref and
+// Acrobat write names, so without them most papers have no outline).
+func (o *outliner) destPage(d reader.Dict) (int, bool) {
+	dest := resolve(o.doc, d.Get("Dest"))
+	if dest == nil {
+		if a, ok := reader.ToDict(resolve(o.doc, d.Get("A"))); ok {
 			if s, ok := reader.ToName(a.Get("S")); ok && s == "GoTo" {
-				if d2 := a.Get("D"); d2 != nil {
-					if arr, ok := reader.ToArray(d2); ok && len(arr) > 0 {
-						if ref, ok := arr[0].(reader.Ref); ok {
-							return ref, true
+				dest = resolve(o.doc, a.Get("D"))
 			}
 		}
 	}
-			}
-		}
-	}
-	return reader.Ref{}, false
+	return o.pageOfDest(dest, 0)
 }

-// pageOf maps a reference to its 1-based page by comparing it with each page's
-// reference.
-func pageOf(doc *reader.Document, ref reader.Ref) (int, bool) {
-	for i := 1; i <= doc.PageCount(); i++ {
-		pr, ok := doc.PageRef(i)
-		if ok && pr == ref {
-			return i, true
+// pageOfDest maps a destination value to its page. depth bounds the chain
+// name → dictionary → array, which a malformed file could make circular.
+func (o *outliner) pageOfDest(dest reader.Object, depth int) (int, bool) {
+	if dest == nil || depth > 4 {
+		return 0, false
 	}
+	switch v := dest.(type) {
+	case reader.Array:
+		if len(v) == 0 {
+			return 0, false
+		}
+		ref, ok := v[0].(reader.Ref)
+		if !ok {
+			return 0, false
+		}
+		return o.pageOf(ref)
+	case reader.Dict:
+		return o.pageOfDest(resolve(o.doc, v.Get("D")), depth+1)
+	case reader.Name:
+		return o.pageOfDest(o.named(string(v)), depth+1)
+	case reader.String:
+		return o.pageOfDest(o.named(string(v)), depth+1)
 	}
 	return 0, false
 }

+// pageOf maps a page reference to its 1-based page number.
+func (o *outliner) pageOf(ref reader.Ref) (int, bool) {
+	if o.pages == nil {
+		o.pages = make(map[reader.Ref]int, o.doc.PageCount())
+		for i := 1; i <= o.doc.PageCount(); i++ {
+			if pr, ok := o.doc.PageRef(i); ok {
+				o.pages[pr] = i
+			}
+		}
+	}
+	p, ok := o.pages[ref]
+	return p, ok
+}
+
+// named returns the destination a name points to, from the PDF 1.1 /Dests
+// dictionary or the /Names /Dests name tree.
+func (o *outliner) named(name string) reader.Object {
+	if o.names == nil {
+		o.names = map[string]reader.Object{}
+		if dests, ok := reader.ToDict(resolve(o.doc, o.cat.Get("Dests"))); ok {
+			for k, v := range dests {
+				o.names[string(k)] = v
+			}
+		}
+		if names, ok := reader.ToDict(resolve(o.doc, o.cat.Get("Names"))); ok {
+			o.nameTree(resolve(o.doc, names.Get("Dests")), map[reader.Ref]bool{}, 0)
+		}
+	}
+	return resolve(o.doc, o.names[name])
+}
+
+// maxNames bounds a name tree read; maxTreeDepth bounds its nesting.
+const (
+	maxNames     = 200000
+	maxTreeDepth = 32
+)
+
+// nameTree reads a name-tree node: leaf /Names pairs [key value …] and /Kids.
+func (o *outliner) nameTree(node reader.Object, seen map[reader.Ref]bool, depth int) {
+	d, ok := reader.ToDict(node)
+	if !ok || depth > maxTreeDepth || len(o.names) >= maxNames {
+		return
+	}
+	if pairs, ok := reader.ToArray(resolve(o.doc, d.Get("Names"))); ok {
+		for i := 0; i+1 < len(pairs) && len(o.names) < maxNames; i += 2 {
+			if k, ok := reader.ToString(resolve(o.doc, pairs[i])); ok {
+				o.names[string(k)] = pairs[i+1]
+			}
+		}
+	}
+	if kids, ok := reader.ToArray(resolve(o.doc, d.Get("Kids"))); ok {
+		for _, k := range kids {
+			if ref, ok := k.(reader.Ref); ok {
+				if seen[ref] {
+					continue
+				}
+				seen[ref] = true
+			}
+			o.nameTree(resolve(o.doc, k), seen, depth+1)
+		}
+	}
+}
+
+// resolve follows a reference. A missing, null or unresolvable object is nil:
+// reader's Dict.Get answers reader.Null{} for an absent key, not nil.
+func resolve(doc *reader.Document, obj reader.Object) reader.Object {
+	if obj == nil {
+		return nil
+	}
+	if _, isNull := obj.(reader.Null); isNull {
+		return nil
+	}
+	v, err := doc.Resolve(obj)
+	if err != nil {
+		return nil
+	}
+	if _, isNull := v.(reader.Null); isNull {
+		return nil
+	}
+	return v
+}
+
 // toRef reports the reference a dictionary entry names, when it is a direct
 // reference rather than an already-resolved object.
 func toRef(o reader.Object) (reader.Ref, bool) {
@@ -148,7 +148,7 @@ func infoTitle(doc *reader.Document) string {
 	if !ok {
 		return ""
 	}
-	return textString(d.Get("Title"))
+	return textString(resolve(doc, d.Get("Title"))) // the title may be an indirect object
 }

 // textString decodes a reader.String (raw bytes) as a PDF text string per rule 3.
@@ -0,0 +1,60 @@
+package pdftext_test
+
+// Owner review (P5a): output over MaxText fails fast, not at the deadline. The child writes more
+// than a pipe buffer (64 KiB), so it blocks on stdout until killed; Run must not wait for it to
+// exit on its own before checking the size.
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"errors"
+	"testing"
+	"time"
+
+	"git.wntrmute.dev/kyle/sift/internal/pdftest"
+	"git.wntrmute.dev/kyle/sift/internal/pdftext"
+)
+
+func TestRunnerTooMuchTextFailsFast(t *testing.T) {
+	var pages []string
+	for p := 1; p <= 80; p++ {
+		pages = append(pages, pdftest.Lorem(p, 40)) // ~2,700 characters a page, ~200 KiB in all
+	}
+	r := runner()
+	r.MaxText = 1000
+	r.Deadline = 30 * time.Second
+	start := time.Now()
+	_, err := r.Run(context.Background(), pdftest.Build(pdftest.Doc{Pages: pages}))
+	if !errors.Is(err, pdftext.ErrTooMuchText) {
+		t.Fatalf("err %v, want ErrTooMuchText", err)
+	}
+	if d := time.Since(start); d > 5*time.Second {
+		t.Errorf("ErrTooMuchText after %v; it waited for the deadline instead of killing the child", d)
+	}
+}
+
+// Owner review (P5a): LaTeX (hyperref) and Acrobat outlines use named destinations through
+// the /Names tree and indirect titles; the first run returned no outline for any arXiv paper
+// or the NIST PDF. reader's Dict.Get answers reader.Null{} for a missing key, which hid /A.
+func TestOutlineNamedDestinations(t *testing.T) {
+	b := pdftest.Build(pdftest.Doc{
+		Title:      "Named",
+		Pages:      []string{"one", "two", "three"},
+		Outline:    []pdftest.Outline{{Title: "1 Intro", Page: 1, Level: 1}, {Title: "2 Method", Page: 3, Level: 1}},
+		NamedDests: true,
+	})
+	var out bytes.Buffer
+	if err := pdftext.Extract(b, 10, &out); err != nil {
+		t.Fatal(err)
+	}
+	var l struct{ Meta pdftext.Meta }
+	line, _ := out.ReadBytes('\n')
+	if err := json.Unmarshal(line, &l); err != nil {
+		t.Fatal(err)
+	}
+	want := []pdftext.OutlineItem{{Title: "1 Intro", Page: 1, Level: 1}, {Title: "2 Method", Page: 3, Level: 1}}
+	if len(l.Meta.Outline) != 2 || l.Meta.Outline[0] != want[0] || l.Meta.Outline[1] != want[1] {
+		t.Errorf("outline = %+v, want %+v", l.Meta.Outline, want)
+	}
+}
@@ -106,14 +106,17 @@ func (r *Runner) Run(ctx context.Context, pdf []byte) (*Doc, error) {
 	}

 	readErr := <-done
-	<-stderrDone

-	// Output over the limit: kill the child and report.
+	// Output over the limit: kill the child at once. Checked before waiting for stderr, which
+	// only closes when the child exits — and a child blocked on a full stdout pipe would not,
+	// until the deadline.
 	if int64(stdoutBuf.Len()) > maxText {
 		cancel()
+		<-stderrDone
 		_ = cmd.Wait()
 		return nil, ErrTooMuchText
 	}
+	<-stderrDone

 	waitErr := cmd.Wait()