Merge remote-tracking branch 'origin/master' into p5a
This commit is contained in:
@@ -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`.
|
||||
@@ -1,150 +0,0 @@
|
||||
# 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.
|
||||
@@ -0,0 +1,89 @@
|
||||
# P5a task 07: the PDF document cache, `pages`, `grep`, and the question limit
|
||||
|
||||
**Branch:** `p5a` (`git switch p5a`; `git status --short` must be empty, otherwise stop)
|
||||
**Commit subject:** `api: PDF cache with revalidation; pages and grep; refuse questions on very long documents`
|
||||
|
||||
## Goal
|
||||
|
||||
Task 06 serves PDFs but fetches and extracts every time. After this task, a PDF already
|
||||
extracted is answered from the cache: fresh for 24 h, then revalidated with a conditional GET.
|
||||
An agent can ask for `pages` or `grep`. A question on a document too long for the digest is
|
||||
refused with a pointer to those two.
|
||||
|
||||
## Files
|
||||
|
||||
- Copy: nothing (the given `internal/api/pdf_test.go` came in task 06)
|
||||
- Modify: `internal/api/pdf.go` (all new logic), `internal/api/api.go` (only the call in rule 1
|
||||
and the two request fields' use in rule 3), `docs/implementer-log.md`
|
||||
|
||||
## What already exists
|
||||
|
||||
- Task 06's `extractPDF`, `pdfError`, `renderPDF`, the `Server` fields (`PDFTTL`,
|
||||
`PDFMaxQuestionChunks`), the `v1Req` fields (`Pages`, `Grep`), and `DefaultPDFTTL` /
|
||||
`DefaultPDFMaxQuestionChunks`.
|
||||
- `internal/dcache`:
|
||||
```go
|
||||
type PDFEntry struct{ ContentHash, ETag, LastModified string; FetchedAt time.Time; Meta []byte; Pages []string }
|
||||
func PDFKey(url, callerClass string) string
|
||||
func (d *DB) PutPDF(key string, e PDFEntry)
|
||||
func (d *DB) GetPDF(key string) (PDFEntry, bool)
|
||||
func (d *DB) TouchPDF(key string, fetchedAt time.Time)
|
||||
```
|
||||
A nil `*DB` is a no-op.
|
||||
- `internal/fetch`: `func (f *Fetcher) FetchIf(ctx, rawURL, etag, lastModified string) (*Resource, error)`.
|
||||
A 304 returns `Status 304`, an empty body and a nil error.
|
||||
- `internal/digest`: `Split(text string, maxTok int) []string` and `ChunkTokens`.
|
||||
|
||||
## Rules
|
||||
|
||||
1. **Cache first, in `doFetch`**, before `acquire` and `s.Fetch.Fetch`: call
|
||||
`s.pdfFromCache(ctx, c, rawURL) (res *fetch.Resource, doc *pdftext.Doc, ok bool, err error)`.
|
||||
The key is `dcache.PDFKey(rawURL, s.callerClass(c))`; the cache is `s.cache()` (may be nil).
|
||||
- **Miss:** `ok == false`; go on as today.
|
||||
- **Fresh** (`time.Since(e.FetchedAt) < ttl`, where `ttl` is `s.PDFTTL` or `DefaultPDFTTL`):
|
||||
build the doc from the entry (unmarshal `Meta` into `doc.Meta`; `doc.Text = e.Pages`), and a
|
||||
synthetic `res`: `URL` and `FinalURL` = rawURL, `Status` 200,
|
||||
`ContentType "application/pdf"`, `FetchedAt` = `e.FetchedAt`, `Hash` = `e.ContentHash`, no
|
||||
body. No network call and no child.
|
||||
- **Stale:** `s.Fetch.FetchIf(ctx, rawURL, e.ETag, e.LastModified)`.
|
||||
- 304 → `TouchPDF(key, now)`, then answer as fresh.
|
||||
- 200 → return that response with `ok == false`, so `doFetch` goes on without a second
|
||||
GET: the new document is extracted and stored.
|
||||
- An error → return it.
|
||||
- The source in the `/v1/fetch` envelope says `"cached": true` for an answer from the cache
|
||||
(fresh or after a 304).
|
||||
2. **Store after extraction:** after `extractPDF` succeeds, `PutPDF` under the URL key with:
|
||||
- `ContentHash: res.SHA256()`;
|
||||
- `ETag` and `Last-Modified` from the response headers;
|
||||
- `FetchedAt: now`;
|
||||
- `Meta`: the JSON of `doc.Meta`;
|
||||
- `Pages: doc.Text`.
|
||||
3. **`pages` and `grep` (`/v1/fetch`):**
|
||||
- Pass `req.Pages` and `req.Grep` into `pdfdoc.Options` in `renderPDF`.
|
||||
- `pdfdoc.ErrBadPages` or `pdfdoc.ErrBadGrep` (use `errors.Is`) → 400, with the error's own
|
||||
text as the message.
|
||||
- `pages` or `grep` on a non-PDF → `400 pages and grep apply to PDFs only`.
|
||||
4. **Question limit:** in mode `digest` with a doc, compute
|
||||
`n := len(digest.Split(pdfdoc.Full(doc), digest.ChunkTokens))`. If `n` exceeds
|
||||
`s.PDFMaxQuestionChunks` (or its default), answer
|
||||
`422 document too long for question (<n> chunks); use grep or pages` **before** any digest
|
||||
call.
|
||||
|
||||
## Steps
|
||||
|
||||
- [ ] **1.** `git switch p5a`. **2. See the three tests fail:**
|
||||
`go test -count=1 -run 'TestPDFPagesAndGrep|TestPDFCache|TestPDFQuestionTooLong' ./internal/api/`.
|
||||
- [ ] **3.** `pdfFromCache` and the store (rules 1–2), **4.** pages and grep, **5.** the question limit.
|
||||
- [ ] **6.** `gofmt -w`; `go test -race -count=3 ./internal/api/` → `ok` (all ten PDF tests and
|
||||
every existing test).
|
||||
- [ ] **7.** `make gate` → `gate: ok`. **8.** Row `p5a/07-api-cache-pages`; 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; the given file is still
|
||||
byte-identical.
|
||||
@@ -1,4 +1,4 @@
|
||||
# P5a task 07: page citations, metrics, CLI flags, skill, unit, README
|
||||
# P5a task 08: 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`
|
||||
@@ -95,7 +95,7 @@ Finish P5a:
|
||||
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.
|
||||
- [ ] **9.** Row `p5a/08-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
|
||||
@@ -14,10 +14,11 @@ vendored by the owner before the run.
|
||||
| 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 | `internal/api` | `api/pdf_test.go` |
|
||||
| 07-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` |
|
||||
| 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 uses 01–05; 07 wires metrics into 06's file.
|
||||
**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
|
||||
@@ -50,3 +51,16 @@ reaches orion until the owner's review, the merge, and straylight's pin bump.
|
||||
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.
|
||||
|
||||
+3
-2
@@ -45,9 +45,10 @@ for task in "$plan"/[0-9][0-9]-*.md; do
|
||||
# shellcheck disable=SC2086
|
||||
# </dev/null: OpenCode 1.15 reads a piped stdin as extra prompt and waits for EOF, so a
|
||||
# backgrounded driver would hang before the first model request (2026-09-25).
|
||||
if ! opencode run -m "$model" $flags --title "$milestone/$name" "$prompt" </dev/null \
|
||||
# timeout: a looping session (P5a task 06 ran 3 h) ends at SESSION_MAX seconds (default 90 min).
|
||||
if ! timeout "${SESSION_MAX:-5400}" opencode run -m "$model" $flags --title "$milestone/$name" "$prompt" </dev/null \
|
||||
> "$runs/$name.log" 2>&1; then
|
||||
echo "run-plan: opencode exited with an error in $name; see $runs/$name.log" >&2
|
||||
echo "run-plan: opencode exited with an error (or hit SESSION_MAX) in $name; see $runs/$name.log" >&2
|
||||
exit 1
|
||||
fi
|
||||
echo "=== $milestone/$name: session ended $(date '+%H:%M:%S')"
|
||||
|
||||
Reference in New Issue
Block a user