kyleandClaude Opus 5.5
218eaa8b53
Kyle: "Fold them into the plan." What changed and why is in docs/P6.md §14. In short:
- FetchFunc gets the *auth.Caller (the given api test could not compile, and a fetch with no
caller would bypass per-caller gates, the pool limit, the cache class and redaction);
- store reads never bump the lease (task 01's rule contradicted its own test); the API Touches;
- 1-2 workers round-robin over live jobs, so a batch holds at most 2 of the 8 global slots;
busy targets retry in the runner (bounded); batch PDFs use their own pool name; 210 s deadline;
- the api tests no longer touch the network or race the cancel; private names via fake DNS;
- failed/cancelled jobs are not reused; GET is a summary plus GET …/{idx} and ?results=1;
- the 256 KiB body cap moves into ServeHTTP; a fixed metrics label for /v1/batch/{id};
- guard.PreCheck (given verbatim) replaces guard.ParseURL at submit;
- 5 tasks became 8 (refactor, wiring, endpoints and serve/config split), each restating the
APIs it uses; every given test compiles against a panic-only skeleton and fails on it.
P6 starts after P5c: task 04 moves v1fetch, which P5b changes. Recheck the api given tests
against post-P5c master before the run.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
59 lines
3.9 KiB
Markdown
59 lines
3.9 KiB
Markdown
# P6 implementation plan: asynchronous batch fetch
|
||
|
||
> **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:** an agent that has collected a series of targets (URLs) and wants them fetched without
|
||
holding a request open per target submits them as one batch, gets a job ID back immediately, and
|
||
polls for per-target results later. A batch is **deliberate and slow** — serial, one target at a
|
||
time — so it is the tool for research threads, not for interactive fetches.
|
||
|
||
Decisions (settled with the owner, 2026-09-26; revised after review the same evening, see
|
||
`docs/P6.md` §14):
|
||
|
||
- **Serial per job, few workers.** One worker by default (at most 2) drains all live jobs,
|
||
round-robin, one target at a time. The batch therefore holds at most 1–2 of the 8 global fetch
|
||
slots, so interactive requests are never starved. A target refused busy (fetch gate or PDF
|
||
pool) is retried after a pause, for up to 10 minutes, then failed.
|
||
- **The caller travels with the job.** The runner keeps each job's `*auth.Caller` in memory and
|
||
hands it to every fetch, so per-caller gates, the PDF pool, the digest cache class and
|
||
restricted-caller redaction all apply. Nothing about the caller is persisted beyond its name
|
||
and class: jobs never resume after a restart.
|
||
- **Caps.** targets/job 32; live jobs/caller 1; live jobs total 8; body 256 KiB. All instrumented
|
||
(`sift_batch_*`).
|
||
- **Idempotency in v1.** The same (caller, target list) returns the existing job while it is live or
|
||
has results (`queued`/`running`/`done`/`partial`) and its lease holds. A failed or cancelled job
|
||
is not reused.
|
||
- **Storage.** `batch_job` + `batch_target` tables in the existing `dcache` SQLite, with its own
|
||
byte budget (64 MiB), outside the content cache's LRU.
|
||
- **Retention / TTL.** One sliding lease per job, bumped on submit, on every target write, and on
|
||
every owner read (the API calls `Touch`; store reads never write). GC deletes a job when its
|
||
lease is older than the TTL (24 h).
|
||
- **Reads.** `GET /v1/batch/{id}` is a summary (statuses, errors). A result is read one at a time
|
||
(`GET /v1/batch/{id}/{idx}`), or all at once with `?results=1`. 32 inline results can be 6 MB.
|
||
- **Restart.** `queued`/`running` jobs are marked `failed` (`restart`) on startup; never resumed.
|
||
- **Restricted callers.** Their fetches go through the same `fetchTarget` with the real caller,
|
||
so the existing redaction applies. No caller can see another's job (404, never 403). Batch
|
||
log lines never carry a target URL or hostname.
|
||
|
||
| Task | Package | Given |
|
||
|---|---|---|
|
||
| 01-batchstore | `internal/dcache` | `dcache/batch_test.go` |
|
||
| 02-runner | `internal/batch` | `batch/runner_test.go` |
|
||
| 03-metrics | `internal/metrics` | `metrics/batch_test.go` |
|
||
| 04-fetchtarget | `internal/api` (the one refactor of working code) | `api/fetchtarget_test.go` |
|
||
| 05-batch-wiring | `internal/api` (+ `guard.PreCheck`, given verbatim) | `api/batchrun_test.go`, `guard/precheck_test.go` |
|
||
| 06-api-endpoints | `internal/api` | `api/batch_test.go` |
|
||
| 07-serve-config | `internal/config`, `cmd/sift/serve.go` | `config/batch_test.go` |
|
||
| 08-cli-docs | `cmd/sift`, README, skill | `cmd_sift/batch_test.go` |
|
||
|
||
Given files live under `_files/<package>/` (`cmd_sift` is `cmd/sift`); each task says where to
|
||
copy its file. Every given test was compiled against a panic-only skeleton of the whole plan and
|
||
fails there on `panic: todo`, not on a compile error.
|
||
|
||
**How this plan was made:** design worked through with the owner from `docs/PLAN.md` §13 (future
|
||
work) on top of the P5 pool (`internal/pdfpool`), cache (`internal/dcache`), and gate
|
||
(`global`/`per` in `internal/api`) infrastructure. Acceptance tests first: the given tests are
|
||
authored against this spec and compiled against a panic-only skeleton so they fail for the
|
||
intended reasons before each task is implemented.
|