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>
87 lines
4.1 KiB
Markdown
87 lines
4.1 KiB
Markdown
# P6 task 04: extract `v1fetch`'s pipeline into `fetchTarget`
|
|
|
|
**Branch:** `p6` (`git switch p6`; `git status --short` must be empty, otherwise stop)
|
|
**Commit subject:** `api: v1fetch's per-target pipeline becomes fetchTarget (no behaviour change)`
|
|
|
|
> This is the one task in P6 that edits working code. It is a **move, not a rewrite**: the
|
|
> reviewer will diff the old `v1fetch` body against `fetchTarget` line by line.
|
|
|
|
## Goal
|
|
|
|
`v1fetch` today inlines the whole per-target pipeline: fetch → recover → PDF render or clean →
|
|
question limit → digest → context → log line → envelope. The batch worker (task 05) needs the
|
|
same pipeline for one target. Move it into one method that both call:
|
|
|
|
```go
|
|
// internal/api/target.go
|
|
func (s *Server) fetchTarget(ctx context.Context, c *auth.Caller, t batch.Target) (batch.Result, error)
|
|
```
|
|
|
|
`v1fetch` keeps its exact responses. **Every existing api test must pass unchanged.**
|
|
|
|
## Files
|
|
|
|
- Copy: `_files/api/fetchtarget_test.go` → `internal/api/fetchtarget_test.go`
|
|
- Create: `internal/api/target.go` (`fetchTarget`)
|
|
- Modify: `internal/api/api.go` (`v1fetch` shrinks; `doFetch` takes a `context.Context` in place of
|
|
the `*http.Request`, and its call sites pass `r.Context()`), `docs/implementer-log.md`
|
|
|
|
## What already exists
|
|
|
|
```go
|
|
// internal/batch (task 02)
|
|
type Options struct{ Mode, Question, Pages, Grep, JQ string; Context *bool; Recover bool }
|
|
type Target struct{ URL string; Options Options }
|
|
type Result struct{ Content string; Meta map[string]any }
|
|
```
|
|
|
|
`fetch.Error{Status int; Msg, Hint string; RetryAfter int}` is the error type `writeFetchErr`
|
|
already turns into a response.
|
|
|
|
## Rules
|
|
|
|
1. **What stays in `v1fetch`:** `s.reqN.Add(1)`, the caller check, decoding `v1Req`, and the
|
|
`400 invalid url` for a bad body or an empty URL. Then it builds a `batch.Target` from the
|
|
request (`URL`, and `Mode`, `Question`, `Pages`, `Grep`, `JQ`, `Context`, `Recover` into
|
|
`Options`), calls `s.fetchTarget(r.Context(), c, t)`, and:
|
|
- on an error, `s.writeFetchErr(w, c, req.URL, err)`;
|
|
- on success, `writeJSON(w, 200, env)` where `env` is `res.Meta` plus `"content": res.Content`.
|
|
2. **What moves into `fetchTarget`, in the same order and unchanged:** the question truncation
|
|
to 500 runes, the mode default and check, `wantCtx`, `doFetch`, the recover path
|
|
(`recover.Try(ctx, …)`), the PDF branch (render, `pages`/`grep` errors, the question-chunk
|
|
limit), the clean/raw branch, the digest, the context search, the `fetch` log line, and the
|
|
envelope.
|
|
3. **Errors that `v1fetch` wrote directly with `writeErr` become `*fetch.Error` values with the same
|
|
status, message and hint:** `400 invalid url` for a bad mode, `400 raw mode does not apply to
|
|
PDFs`. `writeFetchErr` then writes them exactly as before. The only visible difference is its
|
|
`fetch failed` log line, which is fine.
|
|
4. **The envelope:** `fetchTarget` returns `Result{Content: content, Meta: m}` where `m` holds every
|
|
other key of today's `out` map (`mode`, `untrusted`, `source`, `warnings`, `sizes`, `timings`,
|
|
and `pdf`, `digest`, `context` when present). Never put `content` in `Meta`.
|
|
5. **`doFetch(ctx, c, rawURL, jq, onlyMain)`:** `r.Context()` becomes `ctx`. That is the only change
|
|
in `doFetch`. `scrape` (the other caller) passes `r.Context()`.
|
|
6. No new behaviour, no new log fields, no renamed JSON keys. If something cannot move without
|
|
changing behaviour, stop and report it.
|
|
|
|
## Steps
|
|
|
|
- [ ] **1.** `git switch p6`; copy the given test. **2. See it fail** (`fetchTarget` undefined).
|
|
- [ ] **3.** `doFetch(ctx, …)` and its call sites. **4.** `target.go`. **5.** Shrink `v1fetch`.
|
|
- [ ] **6.** `gofmt -w`; `go test -race -count=3 ./internal/api/` → `ok`, every existing test
|
|
included. **7.** `make gate`.
|
|
- [ ] **8.** Row `p6/04-fetchtarget`; commit.
|
|
|
|
```sh
|
|
git add internal/api docs/implementer-log.md
|
|
git commit -F .state/commit-msg.txt
|
|
```
|
|
|
|
## Done when
|
|
|
|
- The two given tests and **every existing api test** pass under `-race -count=3`, with no
|
|
existing test file modified; `make gate` ok; the given file is byte-identical.
|
|
|
|
## Stop and report if
|
|
|
|
- An existing api test fails and fixing it would need a change to that test.
|