From 7b9e1195241fe2f7683a8caa80042823cb2fe3ff Mon Sep 17 00:00:00 2001 From: Claude Lin & Lay Date: Mon, 3 Aug 2026 16:14:09 +0900 Subject: [PATCH] fix(backfill): hold the index cursor at the first uningested candidate [backfill, docs, tests] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bring the #215 watermark invariant to the backfill-issue-index sweep and recalibrate its per-call limits against the 2026-08-03 production measurement. cursor invariant: the cursor is now held one below the first candidate a call failed to ingest, so the next call reopens on that exact number instead of overtaking it. A 404 (absent) is exempt, since nothing will ever ingest those numbers and holding there would stall the sweep permanently. The stall the invariant does introduce is visible in the response (nextCursor equal to the incoming cursor, failed >= 1) and steppable by hand. constants: DEFAULT 25 -> 15 (largest value measured with failed=0), MAX 100 -> 20 (arithmetic only, under the c ~= 40 ceiling of 24). The 100 ceiling was unreachable. Rationale comments now carry the observation boundary and its arithmetic instead of the old ~5 estimate, and record that production re-measurement is no longer available. 予算切れで見送った候補を cursor が追い越していたため、#210 の掃引では limit=25 で一周したあと limit=15 で再掃引する 2 パスが必要だった。cursor を 止めることで、上限値が多少ずれていても取りこぼしがゼロになる — 正しさが定数の 正確さに依存しない形にするのが本体で、定数の実測合わせはその上での器具の精度。 どの binding が見積もりを超えて消費しているかは未特定のまま（断定しない）。 Regression tests: 4 of the 6 new cursor tests fail when the hold is reverted. Closes #216 --- docs/0-requirements.ja.md | 2 +- docs/0-requirements.md | 2 +- docs/installation.ja.md | 18 +++-- docs/installation.md | 18 +++-- src/backfill-issue-index.test.ts | 127 +++++++++++++++++++++++++++++++ src/backfill-issue-index.ts | 112 ++++++++++++++++++++++----- src/index.ts | 10 ++- 7 files changed, 251 insertions(+), 38 deletions(-) diff --git a/docs/0-requirements.ja.md b/docs/0-requirements.ja.md index 07eb68f..5c29f6a 100644 --- a/docs/0-requirements.ja.md +++ b/docs/0-requirements.ja.md @@ -206,7 +206,7 @@ Responsibilities: - commit diff は 1 commit 分の file リストを batch embed（Workers AI の `text: string[]` 対応を利用）し、1 回の Vectorize upsert で N vector を書き込む - batch size は `MAX_EMBEDDING_BATCH_SIZE`（既定 20）で上限。これを超える commit は複数 batch call に分割する -**索引欠落の修復.** watermark の修正は漏れを止めるだけで、既に空いた穴は埋まらない — 取り残された項目が再 fetch されるのは `updated_at` が動いたときだけで、閉じた履歴はもう動かない。`POST /admin/backfill-issue-index?repo=owner/repo`（installation guide 参照）が欠落そのものを走査する。repository の issue 番号空間は密かつ有界なので、`search_docs` に issue / PR 行が無い番号がそのまま欠落集合であり、数値 cursor が「どこまで走査したか」を厳密に表せる。同じ集合を時刻 cursor で辿ると、欠陥が突いた順序をそのまま持ち込むことになる。GitHub 側に既に無い番号（削除・transfer 済み）は 404 を返すので、retry せず計上のみ。取り込みは body-hash 判定を強制的に飛ばす: 候補はいずれも retrieval surface が欠けていると分かっている項目であり、hash が一致していると（embed 成功後に FTS5 mirror が失敗した行がこの状態になる）そのまま恒久的に skip されてしまうため。state 修復と違いこちらは embed を伴うので、1 call の予算は Workers AI の予算であり、呼び出し側が batch を跨いで sweep を進める。`dry_run=true` は予算を使わずに欠落量だけを測る。 +**索引欠落の修復.** watermark の修正は漏れを止めるだけで、既に空いた穴は埋まらない — 取り残された項目が再 fetch されるのは `updated_at` が動いたときだけで、閉じた履歴はもう動かない。`POST /admin/backfill-issue-index?repo=owner/repo`（installation guide 参照）が欠落そのものを走査する。repository の issue 番号空間は密かつ有界なので、`search_docs` に issue / PR 行が無い番号がそのまま欠落集合であり、数値 cursor が「どこまで走査したか」を厳密に表せる。同じ集合を時刻 cursor で辿ると、欠陥が突いた順序をそのまま持ち込むことになる。GitHub 側に既に無い番号（削除・transfer 済み）は 404 を返すので、retry せず計上のみ。取り込みは body-hash 判定を強制的に飛ばす: 候補はいずれも retrieval surface が欠けていると分かっている項目であり、hash が一致していると（embed 成功後に FTS5 mirror が失敗した行がこの状態になる）そのまま恒久的に skip されてしまうため。state 修復と違いこちらは embed を伴い、1 候補ごとに取り込み fan-out の全額がかかるので、呼び出し側が batch を跨いで sweep を進める。`dry_run=true` は予算を使わずに欠落量だけを測る。sweep は poller の watermark と同じ不変条件に従う: **取り込みそこねた最初の候補を cursor が追い越さない。** これにより 1 call の上限を大きく取りすぎたときの代償は 1 回分の無駄な呼び出しであって取りこぼしではなく、結果が「上限値が subrequest 予算をどれだけ正確に写しているか」に依存しなくなる（issue #216）。GitHub 側に既に無い番号は例外 — 誰が何回試しても取り込めないので、そこで止めると retry の境界にならず sweep が停止する。トレードオフは poller と同じで、恒久的に失敗する候補があると sweep は止まる。ただしそれは応答に現れる（`nextCursor` が渡した `cursor` と同じ値で返る）し、この endpoint は cron ではなく人間 / AI が駆動するので、詰まった番号を手動の `cursor` 指定で越えられる。 **取り残した state の修復.** 上の順序欠陥が残した行は、生きている項目として検索に出続ける。通常の poll では到達できない — 差分検出の基準がすでに GitHub と一致しているため。`POST /admin/backfill-issue-state?repo=owner/repo`（installation guide 参照）が repository 単位でこれを揃える。ページングした `state=open` 一覧を正とし、そこに無い索引済み `open` 行を dense / sparse 両側で `closed` にする。再 embed は伴わない（dense 側は既存の値をそのまま再 upsert し、`state` だけ差し替える）。修復は一方向（`open` → `closed`）で、欠陥が生んだ方向に一致する。open 一覧が打ち切られる場合は何もせず中断する — 「一覧に無いこと」が close の根拠だから。 diff --git a/docs/0-requirements.md b/docs/0-requirements.md index 129f458..7cc8b18 100644 --- a/docs/0-requirements.md +++ b/docs/0-requirements.md @@ -207,7 +207,7 @@ Responsibilities: - for commit diffs: batch-embed a commit's file list in a single Workers AI call (`text: string[]`) and upsert the resulting N vectors in one `VECTORIZE.upsert` call - batch size is capped by `MAX_EMBEDDING_BATCH_SIZE` (default 20); commits exceeding it are split across multiple batch calls -**Missing-entry repair.** The watermark fix stops the leak but does not fill the hole: a stranded item is only re-fetched when its `updated_at` moves, and closed history never moves again. `POST /admin/backfill-issue-index?repo=owner/repo` (see the installation guide) walks the gap directly — the repository's issue-number space is dense and bounded, so the numbers with no `search_docs` issue / PR row are exactly the missing set, and a numeric cursor states how far the sweep has reached. A timestamp cursor over the same set would reintroduce the ordering the defect exploited. Numbers GitHub no longer has (deleted or transferred) answer 404 and are counted rather than retried. The ingest is forced past the body-hash check: every candidate is known to be missing a retrieval surface, and a matching hash — which an embed whose FTS5 mirror failed leaves behind — would otherwise skip it permanently. Unlike the state repair this one embeds, so the per-call budget is a Workers AI budget and the caller drives the sweep one batch at a time; `dry_run=true` measures the gap without spending it. +**Missing-entry repair.** The watermark fix stops the leak but does not fill the hole: a stranded item is only re-fetched when its `updated_at` moves, and closed history never moves again. `POST /admin/backfill-issue-index?repo=owner/repo` (see the installation guide) walks the gap directly — the repository's issue-number space is dense and bounded, so the numbers with no `search_docs` issue / PR row are exactly the missing set, and a numeric cursor states how far the sweep has reached. A timestamp cursor over the same set would reintroduce the ordering the defect exploited. Numbers GitHub no longer has (deleted or transferred) answer 404 and are counted rather than retried. The ingest is forced past the body-hash check: every candidate is known to be missing a retrieval surface, and a matching hash — which an embed whose FTS5 mirror failed leaves behind — would otherwise skip it permanently. Unlike the state repair this one embeds, so every candidate carries the full ingest fan-out and the caller drives the sweep one batch at a time; `dry_run=true` measures the gap without spending it. The sweep obeys the same invariant as the poller's watermark: **the cursor never advances past the first candidate a call failed to ingest**, so a per-call limit set too high costs a wasted call rather than a missed item, and the result does not depend on how accurately that limit models the subrequest budget (issue #216). A number GitHub no longer has is exempt — nothing will ever ingest it, so holding there would stall the sweep instead of bounding a retry. The tradeoff is the poller's: a candidate that fails on every attempt stops the sweep. Here that is visible rather than silent (`nextCursor` comes back equal to the `cursor` passed in), and because a human or an AI drives this endpoint rather than cron, stepping over the blocking number is a matter of passing the next `cursor` by hand. **Stale-state repair.** Rows left behind by the ordering defect above keep answering searches as live items, and no ordinary poll reaches them: their diff baseline already matches GitHub. `POST /admin/backfill-issue-state?repo=owner/repo` (see the installation guide) reconciles them per repository — one paginated `state=open` listing supplies the truth, indexed `open` rows absent from it are set to `closed` on both sides, and nothing is re-embedded (the dense side re-upserts the existing values with only `state` replaced). The repair is one-way (`open` → `closed`), which is the direction the defect produced; it aborts rather than act on a truncated open listing, since absence from that listing is what marks a row closed. diff --git a/docs/installation.ja.md b/docs/installation.ja.md index 222ef7c..21c20f8 100644 --- a/docs/installation.ja.md +++ b/docs/installation.ja.md @@ -401,7 +401,7 @@ POST /admin/backfill-issue-index?repo=owner/repo - `repo` — `owner/repo` - `dry_run` — `true` で走査範囲の欠落量だけを測り、GitHub からは何も取得しない -- `limit` — 1 回の呼び出しで試す候補番号の数、`1..100`（既定 `25`）。dry run では無視される +- `limit` — 1 回の呼び出しで試す候補番号の数、`1..20`（既定 `15`）。dry run では無視される - `cursor` — 再開位置の issue 番号。前回のレスポンスの `nextCursor` をそのまま渡す 認証: @@ -411,19 +411,21 @@ POST /admin/backfill-issue-index?repo=owner/repo レスポンス: ```json -{ "repo": "owner/repo", "dryRun": false, "cursor": 0, "limit": 25, "maxNumber": 1690, - "scannedTo": 613, "candidates": 25, "attempted": 25, "indexed": 24, "absent": 1, - "failed": 0, "nextCursor": 613, "done": false } +{ "repo": "owner/repo", "dryRun": false, "cursor": 0, "limit": 15, "maxNumber": 1690, + "scannedTo": 603, "candidates": 15, "attempted": 15, "indexed": 14, "absent": 1, + "failed": 0, "nextCursor": 603, "done": false } ``` 運用上の注意: - まず `dry_run=true` で規模を測る。embedding 予算を使わず、走査した範囲（1 回あたり最大 5000 番）の `candidates` を返す -- `done` が `true` になるまで `nextCursor` を渡して繰り返し呼ぶ。既定の `limit` なら 900 件欠けている repository で 36 回 -- 上の 2 つの修復と違い、この endpoint は **embed する**。`limit` は subrequest 予算であると同時に Workers AI の予算でもあり、100 を超える指定は拒否される +- `done` が `true` になるまで `nextCursor` を渡して繰り返し呼ぶ。既定の `limit` なら 900 件欠けている repository で 60 回 +- 上の 2 つの修復と違い、この endpoint は **embed する**。しかも 1 候補あたりの取り込み fan-out が高価で、実測で 1 invocation の 1000 subrequest のうち約 40 を消費する（issue #216）。既定値はその実測に合わせてある — `15` は本番で `failed: 0` を実測した最大値、`25` は毎回最後の 1 件を落とし、`50` は `Too many subrequests by single Worker invocation` で呼び出し全体が失敗した。`20` を超える指定は拒否される +- `limit` は厳密でなくてよい。**取り込みそこねた最初の候補の 1 つ手前で cursor を止める**ので、次の呼び出しはその番号から再開する（追い越さない）。`limit` が大きすぎたときの代償は 1 回分の無駄な呼び出しであって、取りこぼしではない - 何度実行しても安全。`search_docs` に行がある番号は fetch すらしないので、完了済みの sweep を再実行すると `candidates: 0` が返る。途中で失敗した呼び出しは同じ `cursor` から再開する -- `absent` は GitHub が 404 を返す番号の数（削除された issue、transfer で repository の外に出た番号）。これらは以後の sweep でも候補に残り続けるので、完了した repository でも `candidates` が小さな非ゼロを返すことがある -- `failed` は embed が着地しなかった候補の数。現在の呼び出し内では再試行せず、同じ範囲を次に sweep したときに拾い直す +- `absent` は GitHub が 404 を返す番号の数（削除された issue、transfer で repository の外に出た番号）。これらは以後の sweep でも候補に残り続けるので、完了した repository でも `candidates` が小さな非ゼロを返すことがある。これらで cursor は止めない — 誰が何回試しても取り込めない番号なので、そこで止めると retry の境界にならず sweep が永久に停止する +- `failed` は embed が着地しなかった候補の数。cursor をその最初の 1 件の手前で止めているので、次の呼び出しがまずそれを再試行する +- したがって、毎回必ず失敗する候補があると sweep は止まる。止まったことは応答に現れる — `nextCursor` が渡した `cursor` と同じ値で返り、`failed` が 1 以上になる。手動で越えるには `cursor = nextCursor + 1` を渡す（詰まっている番号は `nextCursor + 1`、run log にも `held before #N` として出る） - 取り込みは body-hash 判定を強制的に飛ばすので、vector はあるが FTS5 行が無い項目も修復される。次の poll を待つのでは代替できない理由がここ - 確認は distinct な番号を数える: `SELECT COUNT(DISTINCT number) FROM search_docs WHERE repo = ? AND type IN ('issue','pull_request')` が実際の issue + PR 件数に近づく diff --git a/docs/installation.md b/docs/installation.md index b13cf2e..a62a7c8 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -401,7 +401,7 @@ Parameters: - `repo` — `owner/repo` - `dry_run` — `true` measures the gap over the scan range and fetches nothing from GitHub -- `limit` — candidate numbers attempted per call, `1..100` (default `25`). Ignored on a dry run +- `limit` — candidate numbers attempted per call, `1..20` (default `15`). Ignored on a dry run - `cursor` — issue number to resume after; pass back the `nextCursor` of the previous response Authentication: @@ -411,19 +411,21 @@ Authentication: Response: ```json -{ "repo": "owner/repo", "dryRun": false, "cursor": 0, "limit": 25, "maxNumber": 1690, - "scannedTo": 613, "candidates": 25, "attempted": 25, "indexed": 24, "absent": 1, - "failed": 0, "nextCursor": 613, "done": false } +{ "repo": "owner/repo", "dryRun": false, "cursor": 0, "limit": 15, "maxNumber": 1690, + "scannedTo": 603, "candidates": 15, "attempted": 15, "indexed": 14, "absent": 1, + "failed": 0, "nextCursor": 603, "done": false } ``` Operational notes: - start with `dry_run=true` to size the job. It spends no embedding budget and reports `candidates` over the range it scanned (up to 5000 numbers per call) -- call it repeatedly, feeding `nextCursor` back in, until `done` is `true`. At the default `limit` a repository missing 900 items takes 36 calls -- this endpoint **embeds**, unlike the two repairs above. `limit` is a Workers AI budget as much as a subrequest budget; raising it above 100 is refused +- call it repeatedly, feeding `nextCursor` back in, until `done` is `true`. At the default `limit` a repository missing 900 items takes 60 calls +- this endpoint **embeds**, unlike the two repairs above, and the ingest fan-out per candidate is expensive: measured at roughly 40 of the invocation's 1000 subrequests (issue #216). The defaults are calibrated to it — `15` is the largest value production completed with `failed: 0`, `25` lost its last candidate on every call, and `50` failed the whole call with `Too many subrequests by single Worker invocation`. Anything above `20` is refused +- the `limit` does not have to be exact. The cursor is **held one below the first candidate the call failed to ingest**, so the next call reopens on that number instead of skipping it — an over-generous `limit` costs a wasted call, not a missing item - safe to repeat: a number that already carries a `search_docs` row is never fetched, so a re-run of a finished sweep reports `candidates: 0`. A call that fails mid-sweep is resumed from the same `cursor` -- `absent` counts numbers GitHub answers 404 for — deleted issues, and numbers whose item was transferred out. They stay candidates on every future sweep, which is why a finished repository still reports a small non-zero `candidates` -- `failed` counts candidates whose embed did not land. They are retried by the next sweep over the same range, not by the current call +- `absent` counts numbers GitHub answers 404 for — deleted issues, and numbers whose item was transferred out. They stay candidates on every future sweep, which is why a finished repository still reports a small non-zero `candidates`. They do not hold the cursor: nothing will ever ingest them, so holding there would stall the sweep rather than bound a retry +- `failed` counts candidates whose embed did not land. The next call retries them first, because the cursor was held below the earliest one +- a candidate that fails on *every* attempt therefore stops the sweep, and says so: `nextCursor` comes back equal to the `cursor` you passed in, with `failed` at 1 or more. Step over it by hand with `cursor = nextCursor + 1` — the blocking number is `nextCursor + 1`, and the run log names it as `held before #N` - the ingest is forced past the body-hash check, so an item whose vector exists but whose FTS5 row is missing is repaired too. This is why the endpoint is not equivalent to waiting for the next poll - verify by counting distinct numbers: `SELECT COUNT(DISTINCT number) FROM search_docs WHERE repo = ? AND type IN ('issue','pull_request')` should approach the repository's real issue + PR count diff --git a/src/backfill-issue-index.test.ts b/src/backfill-issue-index.test.ts index 8c6a3de..89a4af7 100644 --- a/src/backfill-issue-index.test.ts +++ b/src/backfill-issue-index.test.ts @@ -17,6 +17,7 @@ const { backfillIssueIndex, fetchHighestItemNumber, DEFAULT_INDEX_BACKFILL_LIMIT, + MAX_INDEX_BACKFILL_LIMIT, } = await import("./backfill-issue-index.js"); const REPO = "acme/widgets"; @@ -294,6 +295,132 @@ describe("backfill-issue-index: per-call budget", () => { }); }); +describe("backfill-issue-index: the per-call budget constants", () => { + // Production re-measurement is unavailable (the issue #210 sweep took every + // indexed repository to 100% coverage, so no candidates remain), which leaves + // the 2026-08-03 observation plus this assertion as the whole guard. + it("keeps the ceiling below the measured subrequest envelope", () => { + // c ≈ 40 subrequests per candidate against the 1000-subrequest invocation + // budget puts the arithmetic ceiling at 24, and limit=25 was observed to lose + // its last candidate on all 48 production calls. + expect(MAX_INDEX_BACKFILL_LIMIT).toBeLessThan(24); + // 15 is the largest value production completed with failed=0. + expect(DEFAULT_INDEX_BACKFILL_LIMIT).toBeLessThanOrEqual(15); + expect(DEFAULT_INDEX_BACKFILL_LIMIT).toBeLessThanOrEqual(MAX_INDEX_BACKFILL_LIMIT); + }); +}); + +describe("backfill-issue-index: the cursor invariant", () => { + /** Fail exactly the candidates named; everything else lands. */ + function failOn(numbers: number[]) { + processAndUpsertIssueMock.mockImplementation( + async (_e: unknown, _s: unknown, _r: string, issue: { number: number }) => ({ + embedded: !numbers.includes(issue.number), + skippedUnchanged: false, + metadataUpdated: false, + failed: numbers.includes(issue.number), + }), + ); + } + + it("holds the cursor one below the first failed candidate", async () => { + stubGitHub(10, new Set(range(10))); + const { env } = mkEnv([]); + failOn([3]); + + const summary = await backfillIssueIndex(REPO, env, { limit: 5 }); + + // 1..5 attempted, 3 failed: the next call must reopen exactly on 3. + expect(summary.nextCursor).toBe(2); + expect(summary.done).toBe(false); + }); + + it("does not report done while a candidate is left uningested", async () => { + stubGitHub(3, new Set(range(3))); + const { env } = mkEnv([]); + failOn([2]); + + const summary = await backfillIssueIndex(REPO, env, {}); + + // The scan reached the top of the number space, but 2 never landed. + expect(summary.scannedTo).toBe(3); + expect(summary.done).toBe(false); + expect(summary.nextCursor).toBe(1); + }); + + it("does not hold the cursor on a number GitHub no longer has", async () => { + // 2 is absent from GitHub, so no call will ever ingest it; holding there + // would stall the sweep permanently instead of bounding a retry. + stubGitHub(3, new Set([1, 3])); + const { env } = mkEnv([]); + + const summary = await backfillIssueIndex(REPO, env, {}); + + expect(summary.absent).toBe(1); + expect(summary.done).toBe(true); + expect(summary.nextCursor).toBeNull(); + }); + + it("makes a stall visible as a cursor that did not move", async () => { + stubGitHub(10, new Set(range(10))); + const { env } = mkEnv([]); + failOn([6]); + + const summary = await backfillIssueIndex(REPO, env, { limit: 3, cursor: 5 }); + + expect(summary.failed).toBe(1); + expect(summary.nextCursor).toBe(5); + }); + + it("steps over a permanently failing number when the caller advances the cursor", async () => { + const { fetched } = stubGitHub(10, new Set(range(10))); + const { env } = mkEnv([]); + failOn([6]); + + // Recovery path: cursor = nextCursor + 1 of the stalled call above. + const summary = await backfillIssueIndex(REPO, env, { limit: 3, cursor: 6 }); + + expect(fetched).toEqual([7, 8, 9]); + expect(summary.failed).toBe(0); + expect(summary.nextCursor).toBe(9); + }); + + it("loses nothing across calls when the per-call budget cuts off mid-batch", async () => { + const { env } = mkEnv([]); + const landed: number[] = []; + let attemptsThisCall = 0; + + // Budget model taken from the 2026-08-03 measurement: the last candidate of + // an over-large call is the one the subrequest budget cuts off. + processAndUpsertIssueMock.mockImplementation( + async (_e: unknown, _s: unknown, _r: string, issue: { number: number }) => { + attemptsThisCall++; + if (attemptsThisCall >= 3) { + return { embedded: false, skippedUnchanged: false, metadataUpdated: false, failed: true }; + } + landed.push(issue.number); + return { embedded: true, skippedUnchanged: false, metadataUpdated: false, failed: false }; + }, + ); + + let cursor: number | null = 0; + let calls = 0; + while (cursor !== null && calls < 20) { + stubGitHub(10, new Set(range(10))); + attemptsThisCall = 0; + const summary: Awaited> = + await backfillIssueIndex(REPO, env, { limit: 3, cursor }); + cursor = summary.nextCursor; + calls++; + } + + // Every number lands exactly once, in order, despite every call losing its + // third candidate to the budget. + expect(landed).toEqual(range(10)); + expect(cursor).toBeNull(); + }); +}); + describe("backfill-issue-index: dry run", () => { it("measures the gap over the whole scan range without fetching or writing", async () => { const { fetched } = stubGitHub(10, new Set(range(10))); diff --git a/src/backfill-issue-index.ts b/src/backfill-issue-index.ts index 9571653..1a1e616 100644 --- a/src/backfill-issue-index.ts +++ b/src/backfill-issue-index.ts @@ -19,22 +19,60 @@ * cursor over the same set would reintroduce the ordering the defect exploited. * * Unlike `./backfill-issue-state.ts`, this repair does embed: a missing row has - * no vector to re-upsert. The per-call budget is therefore a Workers AI budget - * first, and the caller drives the sweep one batch at a time. + * no vector to re-upsert, so every candidate carries the full ingest fan-out and + * the caller drives the sweep one batch at a time. What that fan-out actually + * costs per candidate is measured at `DEFAULT_INDEX_BACKFILL_LIMIT` below. */ import type { Env } from "./types.js"; import { processAndUpsertIssue, type GitHubIssueData } from "./pipeline.js"; -/** Candidate numbers attempted per call, unless the caller lowers it. Each one - * costs a GitHub fetch plus (when present) an embed + Vectorize + D1 + store - * fan-out, ~5 of the invocation's 1000 subrequests. */ -export const DEFAULT_INDEX_BACKFILL_LIMIT = 25; +/** + * Per-candidate subrequest cost, measured rather than estimated. + * + * Observed in production on 2026-08-03 while sweeping issue #210, over 4 + * repositories and 48 calls: + * + * - `limit=50` -> the *first* call of every repository fails outright with + * `Too many subrequests by single Worker invocation` + * - `limit=25` -> every one of the 48 calls reports `indexed=24 failed=1`. + * Exactly one, every time, across repositories — the budget runs out on the + * 25th candidate, not on any property of the item + * - `limit=15` -> `failed=0` on all 8 calls + * + * The 24 / 25 boundary is what the cost is read off. With a 1000-subrequest + * invocation budget, per-candidate cost `c`, and per-call fixed cost `O` + * (`fetchHighestItemNumber` plus the scan-chunk D1 queries): + * + * 24c + O <= 1000 < 25c + O => c ≈ 40 (39.2 .. 41.7 for O in 0..20) + * + * ~40 subrequests per candidate, against ~6 logical binding calls per candidate + * in this module's code path (GitHub fetch 1 / store DO 2 / Workers AI 1 / + * Vectorize 1 / D1 1). One logical call therefore does not equal one + * subrequest here, and **which binding accounts for the difference is not + * identified** — do not infer a mechanism from these numbers, only a budget. + * `MAX_COMMENT_FETCHES_PER_REPO_PER_RUN` in `./poller.ts` carries the same + * shape of correction, from `~3-5` down to a measured value. + * + * These figures cannot be re-measured: the #210 sweep took all 6 indexed + * repositories to 100% coverage, so no candidates remain to consume budget. + * Deleting rows to manufacture candidates is not an option. Any later revision + * of these constants rests on the data above plus the unit tests, and the + * cursor invariant below is what keeps correctness independent of their + * accuracy. + */ + +/** Candidate numbers attempted per call, unless the caller overrides it. + * 15 is the largest value **measured** to complete with `failed=0`. */ +export const DEFAULT_INDEX_BACKFILL_LIMIT = 15; -/** Hard ceiling on the per-call candidate budget. 100 candidates x ~5 subrequests - * ≈ 500, half the per-invocation budget, and 100 embeds is twice the cron's own - * per-run Workers AI allowance. */ -export const MAX_INDEX_BACKFILL_LIMIT = 100; +/** Hard ceiling on the per-call candidate budget. At c ≈ 40 the arithmetic + * ceiling is 1000 / 40 ≈ 24, so 20 keeps ~15% headroom under it. 20 is + * **arithmetic only — never measured**; 15 (the default above) is the measured + * value. The previous ceiling of 100 was unreachable: any call near it failed + * as a whole, which is the kind of limit that drops the next operator into the + * same hole. */ +export const MAX_INDEX_BACKFILL_LIMIT = 20; /** Issue numbers covered by one indexed-set query. */ const SCAN_CHUNK = 200; @@ -71,9 +109,12 @@ export interface IssueIndexBackfillSummary { indexed: number; /** Candidates GitHub does not have (deleted or transferred numbers). */ absent: number; - /** Candidates whose embed or upsert failed; retried by a later call. */ + /** Candidates whose embed or upsert failed. The cursor is held below the first + * of them, so the next call resumes on that exact number. */ failed: number; - /** Pass back as `cursor` to continue; `null` once the sweep reached `maxNumber`. */ + /** Pass back as `cursor` to continue; `null` once the sweep reached `maxNumber` + * with every candidate ingested. Equal to the `cursor` that was passed in when + * the call's first candidate failed — the sweep is held, not finished. */ nextCursor: number | null; done: boolean; } @@ -170,6 +211,26 @@ async function fetchItem( * and re-running a batch re-issues writes that are already correct. Call it * repeatedly with the returned `nextCursor` until `done`. * + * Cursor invariant, the same one the poller's watermark obeys (`nextIssueWatermark` + * in `./poller.ts`, issue #210): **the cursor never advances past the first + * candidate this call failed to ingest.** Candidates are walked in ascending + * number order, so holding at the first failure covers every later one. Without + * the hold, a candidate the subrequest budget cut off was overtaken by the cursor + * and only a second sweep at a lower `limit` picked it up (issue #216) — which is + * exactly what production needed on the #210 repair. With it, an over-generous + * `limit` costs a wasted call, not a missed item: correctness stops depending on + * how accurate `DEFAULT_INDEX_BACKFILL_LIMIT` is. + * + * A 404 (`absent`) does not hold the cursor. Those numbers are permanently gone + * from GitHub, so holding on one would stall the sweep forever rather than + * bounding a retry. + * + * The tradeoff is the poller's: a candidate that fails on every attempt stops the + * sweep. Here it is visible rather than silent — `nextCursor` comes back equal to + * the `cursor` that went in, with `failed >= 1` — and this endpoint is driven by a + * human or an AI, not by cron. Pass `cursor = nextCursor + 1` to step over the + * blocking number by hand. + * * The ingest is forced past the body-hash check. The hash answers "did the body * change", but every candidate here is known to be missing a retrieval surface, * which a matching hash would otherwise skip over permanently. @@ -213,6 +274,8 @@ export async function backfillIssueIndex( let indexed = 0; let absent = 0; let failed = 0; + /** First candidate number this call did not get onto the retrieval surfaces. */ + let retryBoundary: number | undefined; if (!dryRun) { const storeId = env.ISSUE_STORE.idFromName("global"); @@ -228,17 +291,32 @@ export async function backfillIssueIndex( const result = await processAndUpsertIssue(env, storeStub, repo, item, { force: true, }); - if (result.embedded) indexed++; - else failed++; + if (result.embedded) { + indexed++; + } else { + failed++; + retryBoundary ??= number; + } } } - const done = scannedTo >= maxNumber; + // Held one below the first uningested candidate so the next call reopens on it. + // `retryBoundary - 1 >= cursor` always holds (every candidate is above `cursor`), + // so the cursor cannot regress; equality is the stall signal. + const nextCursor = + retryBoundary !== undefined + ? retryBoundary - 1 + : scannedTo >= maxNumber + ? null + : scannedTo; + const done = nextCursor === null; console.log( `${repo} backfill-issue-index: max=${maxNumber} cursor=${cursor} ` + `scanned_to=${scannedTo} candidates=${candidates.length} attempted=${attempted} ` + - `indexed=${indexed} absent=${absent} failed=${failed}${dryRun ? " (dry run)" : ""}`, + `indexed=${indexed} absent=${absent} failed=${failed}` + + `${retryBoundary !== undefined ? ` (held before #${retryBoundary})` : ""}` + + `${dryRun ? " (dry run)" : ""}`, ); return { @@ -253,7 +331,7 @@ export async function backfillIssueIndex( indexed, absent, failed, - nextCursor: done ? null : scannedTo, + nextCursor, done, }; } diff --git a/src/index.ts b/src/index.ts index 6f1ff0f..18ba5fc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -514,11 +514,15 @@ const innerHandler: ExportedHandler = { // POST /admin/backfill-issue-index?repo=owner/repo[&dry_run=true][&limit=N][&cursor=N] // Fills the gap the poller's watermark left behind (issue #210). Walks the repo's // issue-number space, finds the numbers with no `search_docs` issue / PR row, and - // ingests them. Unlike `/admin/backfill-issue-state` this embeds, so the per-call - // budget is a Workers AI budget; unlike `/admin/reset-hashes` it re-embeds only the + // ingests them. Unlike `/admin/backfill-issue-state` this embeds, so every candidate + // carries the full ingest fan-out (~40 subrequests, measured — see + // `DEFAULT_INDEX_BACKFILL_LIMIT`); unlike `/admin/reset-hashes` it re-embeds only the // missing items rather than the whole repository. // `dry_run=true` measures the gap over the scan range without spending either budget. - // Call repeatedly, passing the returned `nextCursor` back, until `done` is true. + // Call repeatedly, passing the returned `nextCursor` back, until `done` is true. The + // cursor is held below the first candidate a call failed to ingest, so an unchanged + // `nextCursor` with `failed >= 1` means the sweep is blocked on that number (issue + // #216); pass `cursor = nextCursor + 1` to step over it. // Requires GITHUB_TOKEN header for authentication. if (request.method === "POST" && url.pathname === "/admin/backfill-issue-index") { const authHeader = request.headers.get("GITHUB_TOKEN");