diff --git a/docs/0-requirements.ja.md b/docs/0-requirements.ja.md index 5c29f6a..0ef8984 100644 --- a/docs/0-requirements.ja.md +++ b/docs/0-requirements.ja.md @@ -136,9 +136,17 @@ Responsibilities: 各 invocation は独立した subrequest 予算を持つ。dispatch は `controller.cron` で `handleScheduled` 内で行う。未知の cron 表現は no-op log で silent regression を防止する。 +**ETag の据え置き.** 条件付き request を出す surface は 3 つある — issue / PR(issues list への `If-None-Match`)、releases(releases list)、docs(repository tree)— そしてこの 3 面は 1 つの規則に従う: **やり残しがある run は、受け取った ETag を保存しない。** やり残しとは retrieval surface に載らなかった項目のことで、理由は問わない — per-run cap が見送った場合も、embed が失敗した場合も同じ。どちらでも項目には retry の印が付き(空の `bodyHash`、または store 側で進んでいない `blobSha`)、次 run がその項目を見に来ることが前提になる。ところが ETag を保存すると、次 run の条件付き request は 304 を返して項目を見る前に return してしまい、印の付いた項目は同じ surface の**無関係な**項目が変更されるまで待たされる。`lastPolledAt` は進めるので、据え置いた run 自体は観測できる。 + +条件を 3 面それぞれに持たせず 1 つに揃えているのは、面ごとの条件が離れていくからである。releases 面は upsert cap だけを、docs 面は 2 本の cap だけを見ていたため、どちらも embed 失敗では ETag を更新してしまい retry が止まっていた(issue #211)。この一般形に最初に到達したのは issue / PR 面(issue #215)。*据え置き*の実装は releases / docs 面が「直前に保存した ETag を保持」、issue / PR 面が「何も保存しない」だが、この 2 つは等価である。保存済み ETag を送って 200 が返った run は、その surface が既にその ETag と異なることを証明しているので、次の request はどちらの形でも 200 になる。 + +残りの取り込み surface がこの規則の外にあるのは、書き漏らしではなく構造上の理由である。comment poller は条件付き request を出さない(毎 run 直近の parent を歩き直す)。wiki poller の `etag` watermark 列が持っているのは HTTP ETag ではなく walk の cursor である。 + issue / PR poller は `(lastPolledAt, now]` を `updated_at` 昇順で取得する。1 run あたり最大 `MAX_PAGES_PER_RUN` × 100 = 200 件を fetch し、そのうち embed するのは最大 `MAX_EMBEDDINGS_PER_RUN` = 50 件。この surface は commit diff と同じ watermark 不変条件に従う: **その run が retrieval surface に載せられなかった最も古い項目を watermark が追い越さない** — embedding 予算で見送った項目と、embed に失敗した項目の両方が対象。2 つの境界は別物として扱う。fetch がどこまで届いたか(poll 開始時刻、pagination が打ち切られた場合は最後に fetch した項目の `updated_at`)は上限にすぎず、取り込み境界がその下に watermark を留める。留める位置は境界の 1 秒手前で、GitHub の `since` filter が境界の項目自身を再び含むようにするため。 -この pin が無い間、この surface は「fetch 件数 − embed 件数」の速度で取りこぼしていた。旧実装の watermark はどちらの分岐でも見送った項目より**新しい**位置に着地する — batch は昇順なので、予算はいつもその新しい端で尽きる — 一方で見送った項目に付く空の `bodyHash` は retry の印であって、以後どの `since` window もその項目を fetch しない。2026-08-03 の実測で、索引対象 repository の issue / pull request 履歴の約 55% が索引に載っていなかった。欠落が連続した番号帯ではなく散発に見えるのは、脱落が番号順ではなく `updated_at` 順に起きるため(issue #210)。**ETag も同じ条件で書き戻さない。** docs poller が tree ETag を保持するのと同じ理由で、ETag を保存すると次 run の条件付き request が 304 を返し、残りを見る前に return してしまう。 +この pin が無い間、この surface は「fetch 件数 − embed 件数」の速度で取りこぼしていた。旧実装の watermark はどちらの分岐でも見送った項目より**新しい**位置に着地する — batch は昇順なので、予算はいつもその新しい端で尽きる — 一方で見送った項目に付く空の `bodyHash` は retry の印であって、以後どの `since` window もその項目を fetch しない。2026-08-03 の実測で、索引対象 repository の issue / pull request 履歴の約 55% が索引に載っていなかった。欠落が連続した番号帯ではなく散発に見えるのは、脱落が番号順ではなく `updated_at` 順に起きるため(issue #210)。**ETag も同じ条件で書き戻さない**(上記「ETag の据え置き」に従う)。 + +releases poller は releases list 全体を 1 回の条件付き request で読み、1 repo 1 run あたり最大 `MAX_RELEASE_UPSERTS_PER_REPO_PER_RUN` = 10 件を upsert する。list は有界で毎回全件読み直すため、この面は watermark を持たない。したがって ETag の据え置きがこの面唯一のやり残し機構である。cap が見送った release も、embed に失敗した release も、どちらも空の `bodyHash` で保存され、次 run で拾えるのは ETag を据え置いたからにすぎない(issue #149 / #211)。 commit diff poller は 2-phase 構成: @@ -157,7 +165,9 @@ commit diff poller は 2-phase 構成: docs poller は `If-None-Match` 付きの条件付きリクエストで repository tree を読み、保存済みの doc record と差分を取る。blob SHA が動いた entry は re-embed し、store にあって tree に無い entry は削除する。**削除は 3 面を teardown する** — Vectorize / D1 FTS5 / structured store — それぞれ独立に実行するので、Vectorize の失敗が実際に retrieval される D1 行を取り残すことはない。wiki 側の 4 面に対してここが 3 面なのは、doc vector ID が `doc_edges` の端点になりえないため。`indexWikiEdges` が唯一の writer であり、src 側も算出される dst 側も wiki vector ID になる。この不変条件が変わったら、ここに edge の teardown を足すこと(issue #203)。なお削除が届くのは現行の vector ID 世代だけで、移行前世代は構造上到達できず、別経路で掃除する(Vector Store の「移行前世代」を参照)。 -**削除の枠.** 削除は 1 repo 1 run あたり `MAX_DOC_DELETIONS_PER_REPO_PER_RUN`(既定 5)で cap する。wiki の削除と同じ guard で、1 件あたり 3 subrequest かかるため、大量削除に対して上限なく回すと light cron の invocation 予算を単独で食い潰し、後ろに並ぶ repo を飢えさせうる — 1 つの PR が `.md` を 66 件削除すれば、その全件が 1 run に集中する。削除済み doc の store 行は消えるので残りの集合は縮む一方であり、drain は単調。したがってこの surface には per-run cap が 2 本あり、**どちらが効いても tree ETag は据え置く**: fetch 枠は未処理の変更 doc を残し(issue #149)、削除枠は未削除の doc を残す(issue #203)。どちらの場合も ETag を進めてしまうと次 run が 304 で返り、残りを見ないまま終わる。`lastPolledAt` は進めるので run 自体は観測できる。 +**削除の枠.** 削除は 1 repo 1 run あたり `MAX_DOC_DELETIONS_PER_REPO_PER_RUN`(既定 5)で cap する。wiki の削除と同じ guard で、1 件あたり 3 subrequest かかるため、大量削除に対して上限なく回すと light cron の invocation 予算を単独で食い潰し、後ろに並ぶ repo を飢えさせうる — 1 つの PR が `.md` を 66 件削除すれば、その全件が 1 run に集中する。削除済み doc の store 行は消えるので残りの集合は縮む一方であり、drain は単調。したがってこの surface にはやり残しの経路が 3 本あり、**どれが効いても tree ETag は据え置く**(上記「ETag の据え置き」に従う): fetch 枠は未処理の変更 doc を残し(issue #149)、削除枠は未削除の doc を残し(issue #203)、embed 失敗は store 行が古い `blobSha` を保持したままの doc を残す(issue #211)。 + +docs / releases のどちらの loop も、自前の embedding cap を持たない。両者の fan-out cap(各 10)が `MAX_EMBEDDINGS_PER_RUN` = 50 より小さく、embed 件数を構造的に抑えているため、かつて置かれていた guard は到達不能だった。到達不能な分岐は「それも ETag の据え置きを負うべきか」を読み手が判断できない状態を残し、issue #211 の穴が欠陥ではなく曖昧さとして読めてしまう原因になっていた。**この定数関係はテストで検証している**——各 cap の位置のコメントに委ねてはいない。この repo の fan-out cap は実際に調整される(issue #134 は comment fetch cap を 30 から 10 に下げた)ので、50 より上に引き上げた時に、その面の embed 上限が黙って消えてしまうからである。assertion が落ちることは「新しい cap が誤り」という判定ではない。その loop に embedding guard を戻し、見送った項目が ETag を据え置けるよう、その loop の ETag 据え置き条件に組み込む必要がある、という意味である。 wiki poller は `:45` cron 専属で、GitHub Wiki content の唯一の取り込み経路。Wiki は別 git repo (`{repo}.wiki.git`) に存在し、REST API も webhook event も持たないため、poller が repo ごとに 3 段の HTTP 呼び出しで処理する: diff --git a/docs/0-requirements.md b/docs/0-requirements.md index 7cc8b18..443ff98 100644 --- a/docs/0-requirements.md +++ b/docs/0-requirements.md @@ -137,9 +137,17 @@ The poller runs hourly in the current deployment, split across four cron trigger Dispatch is performed inside `handleScheduled` by inspecting `controller.cron`. Unknown cron expressions fall through to a no-op log to prevent silent regressions when triggers are added later. +**The ETag hold.** Three surfaces make conditional requests — issue / PR (`If-None-Match` on the issues list), releases (on the releases list), and docs (on the repository tree) — and one rule governs all three: **a run that left work behind does not store the ETag it just received.** Work is left behind whenever an item did not reach the retrieval surfaces, whatever the reason — a per-run cap deferred it, or its embed failed. Either way the item carries a retry marker (an empty `bodyHash`, or a `blobSha` the store never advanced) and needs the next run to look at it again; a stored ETag makes that run's conditional request answer 304 and return before it does, leaving the marked item to wait for some unrelated item on the same surface to change. `lastPolledAt` still advances, so a holding run stays observable. + +The rule is deliberately one condition rather than three, because per-surface conditions drift apart: the releases branch watched its upsert cap alone and the docs branch watched its two caps, so on both an embed failure still refreshed the ETag and stalled the retry (issue #211). The issue / PR surface reached the general form first (issue #215). *Holding* is expressed as "keep the previously stored ETag" on the releases and docs surfaces and as "store none" on the issue / PR surface; the two are equivalent, because a run that received a 200 for the stored ETag has proved the surface already differs from it, so the next request is answered 200 either way. + +The remaining ingestion surfaces are outside this rule by construction, not by omission: the comment poller issues no conditional request (it re-walks the most recent parents every run), and the wiki poller's `etag` watermark column holds its walk cursor rather than an HTTP ETag. + The issue / PR poller fetches `(lastPolledAt, now]` sorted by `updated_at` ascending, at most `MAX_PAGES_PER_RUN` × 100 = 200 items, and embeds at most `MAX_EMBEDDINGS_PER_RUN` = 50 of them. It obeys the same watermark invariant as the commit-diff surface: **the watermark never advances past the earliest item the run left off the retrieval surfaces** — deferred by the embedding budget, or failed to embed. The two bounds are separate: how far the fetch reached (the poll start time, or the last fetched item when pagination capped) is an upper bound, and the ingest boundary pins the watermark below it, one second earlier so GitHub's `since` filter still re-includes the boundary item. -Without that pin the surface leaked at a rate of fetched-minus-embedded per run. Both of the old watermark branches landed *above* every deferred item — the batch is ascending, so the budget always runs out on its newest end — and the deferred item's empty `bodyHash` marked it for a retry that no later `since` window would ever fetch. Measured 2026-08-03, that left about 55% of the issue and pull request history of the indexed repositories absent from the index, scattered rather than in contiguous ranges because the loss follows `updated_at` order and not number order (issue #210). **The ETag is withheld on the same condition**, for the reason the docs poller withholds its tree ETag: a stored ETag makes the next run's conditional request answer 304 and return before it looks at the leftover. +Without that pin the surface leaked at a rate of fetched-minus-embedded per run. Both of the old watermark branches landed *above* every deferred item — the batch is ascending, so the budget always runs out on its newest end — and the deferred item's empty `bodyHash` marked it for a retry that no later `since` window would ever fetch. Measured 2026-08-03, that left about 55% of the issue and pull request history of the indexed repositories absent from the index, scattered rather than in contiguous ranges because the loss follows `updated_at` order and not number order (issue #210). **The ETag is withheld on the same condition**, per the ETag hold above. + +The releases poller reads the whole releases list in one conditional request and upserts at most `MAX_RELEASE_UPSERTS_PER_REPO_PER_RUN` = 10 of them per repo per run. It needs no watermark of its own — the list is bounded and re-read in full — so the ETag hold is its only leftover mechanism: releases the cap deferred, and releases whose embed failed, are both stored with an empty `bodyHash` and are reachable next run only because the ETag was held (issue #149 / #211). The commit-diff poller runs in two phases: @@ -158,7 +166,9 @@ The tradeoff is liveness: a commit that fails on every attempt blocks its phase' The docs poller reads the repository tree with a conditional `If-None-Match` request and diffs it against the stored doc records: entries whose blob SHA moved are re-embedded, entries present in the store but absent from the tree are reaped. **The reap tears down three surfaces** — Vectorize, D1 FTS5, and the structured store — each independently, so a Vectorize failure cannot strand the D1 rows users actually retrieve. It is three and not the wiki reap's four because a doc vector ID is never a `doc_edges` endpoint: `indexWikiEdges` is the only writer and both the source and the computed destination IDs are wiki vector IDs. Add the edge teardown here if that invariant changes (issue #203). The reap reaches only the current vector-ID generation; the pre-migration generation is unreachable by construction and is cleaned out separately (see Vector Store, "Pre-migration generation"). -**Reap budget.** The reap is capped at `MAX_DOC_DELETIONS_PER_REPO_PER_RUN` (default 5) per repo per run, the same guard the wiki reap carries: at 3 subrequests per deletion an unbounded loop over a mass deletion could exhaust the light cron's invocation budget on its own and starve every repo behind it — a single PR removing 66 `.md` files puts all of them on one run. The drain is monotonic, since a reaped doc's store row is gone and the leftover set only shrinks. Two per-run caps therefore exist on this surface, and **either one holds the tree ETag back**: the fetch cap leaves changed docs unprocessed (issue #149) and the delete cap leaves deletions unreaped (issue #203), and in both cases advancing the ETag would make the next run answer 304 and return before it looked at the leftover. `lastPolledAt` still advances so the run stays observable. +**Reap budget.** The reap is capped at `MAX_DOC_DELETIONS_PER_REPO_PER_RUN` (default 5) per repo per run, the same guard the wiki reap carries: at 3 subrequests per deletion an unbounded loop over a mass deletion could exhaust the light cron's invocation budget on its own and starve every repo behind it — a single PR removing 66 `.md` files puts all of them on one run. The drain is monotonic, since a reaped doc's store row is gone and the leftover set only shrinks. This surface therefore has three ways to leave work behind, and **any one of them holds the tree ETag back** per the ETag hold above: the fetch cap leaves changed docs unprocessed (issue #149), the delete cap leaves deletions unreaped (issue #203), and a failed embed leaves a doc whose store row still carries the old `blobSha` (issue #211). + +Neither the docs nor the releases loop carries an embedding cap of its own. Their fan-out caps (10 each) sit below `MAX_EMBEDDINGS_PER_RUN` = 50 and bound the embed count by construction, so the guards those loops used to hold were unreachable — and unreachable code left the reader unable to tell whether it also owed the ETag hold, which is how the issue #211 hole read as ambiguous rather than as a defect. **That constant relation is asserted by a test**, not left to the comments at the cap sites: fan-out caps on this repo get retuned (issue #134 took the comment fetch cap from 30 to 10), and raising one past 50 would otherwise remove the surface's embed bound in silence. A failing assertion is not a verdict that the new cap is wrong — it says the loop now needs its embedding guard back, folded into that loop's ETag-hold condition so a deferred item still holds the ETag. The wiki poller runs in the `:45` cron and is the only ingestion path for GitHub Wiki content. Wiki pages live in a separate git repo (`{repo}.wiki.git`) that GitHub does not expose through the REST API or webhook events; the poller therefore performs three lightweight HTTP calls per repo: diff --git a/src/poller-docs.test.ts b/src/poller-docs.test.ts index a534bb3..af6375b 100644 --- a/src/poller-docs.test.ts +++ b/src/poller-docs.test.ts @@ -28,6 +28,7 @@ const { docVectorId } = await import("./pipeline.js"); const REPO = "acme/widgets"; const WATERMARK_KEY = `docs:${REPO}`; const TREE_URL = `https://api.github.com/repos/${REPO}/git/trees/HEAD?recursive=1`; +const CONTENTS_PREFIX = `https://api.github.com/repos/${REPO}/contents/`; /** Constant mirrored from `src/poller.ts`. */ const DELETE_BUDGET = 5; @@ -39,11 +40,22 @@ const DELETE_BUDGET = 5; * the conditional-request contract by returning 304 when the poller sends back * the matching `If-None-Match`. */ -function stubTree(paths: string[], etag = 'W/"tree-1"') { +function stubTree( + paths: string[], + etag = 'W/"tree-1"', + opts: { failContent?: boolean } = {}, +) { const conditionalHits: string[] = []; + /** Doc paths whose content the poller actually fetched this run. */ + const contentFetches: string[] = []; const fetchMock = vi.fn(async (input: string | URL, init?: RequestInit) => { const url = String(input); + if (url.startsWith(CONTENTS_PREFIX)) { + contentFetches.push(url.slice(CONTENTS_PREFIX.length)); + if (opts.failContent) throw new Error("simulated Contents API outage"); + return Response.json({ content: btoa("# doc body"), encoding: "base64" }); + } if (url !== TREE_URL) { throw new Error(`unexpected fetch in docs stub: ${url}`); } @@ -64,7 +76,7 @@ function stubTree(paths: string[], etag = 'W/"tree-1"') { }); vi.stubGlobal("fetch", fetchMock); - return { fetchMock, conditionalHits }; + return { fetchMock, conditionalHits, contentFetches }; } /** @@ -143,6 +155,13 @@ const stored = (path: string): DocRecord => ({ updatedAt: "2026-08-01T00:00:00Z", }); +/** A stored doc whose blob SHA is stale, so the poller sees it as changed and + * routes it into the embed path. */ +const staleStored = (path: string): DocRecord => ({ + ...stored(path), + blobSha: "blob-outdated", +}); + beforeEach(() => { processAndUpsertDocMock.mockReset(); processAndUpsertDocMock.mockResolvedValue({ embedded: true, failed: false }); @@ -285,3 +304,70 @@ describe("poller: pollDocs delete budget", () => { expect(store.etag()).toBe('W/"tree-1"'); }); }); + +describe("poller: pollDocs ETag hold on embed failure", () => { + // The hole issue #211 closed. Both per-run caps held the tree ETag back, but a + // failed embed did not — and a failed embed leaves exactly the same leftover: a + // doc still absent from the retrieval surfaces. Storing the fresh ETag made the + // next run answer 304 and return before `changedEntries` was even computed, so + // the doc waited for some *other* file in the repo to change. + const PRIOR_ETAG = 'W/"tree-prior"'; + + it("holds the prior ETag when a doc failed to embed", async () => { + stubTree(["docs/broken.md"]); + const store = makeDocStore([staleStored("docs/broken.md")], PRIOR_ETAG); + const { env } = makeDocEnv(); + processAndUpsertDocMock.mockResolvedValue({ embedded: false, failed: true }); + + await pollDocs(REPO, env, store.stub); + + expect(processAndUpsertDocMock).toHaveBeenCalledTimes(1); + expect(store.etag()).toBe(PRIOR_ETAG); + }); + + it("holds the prior ETag when the content fetch threw", async () => { + // The `catch` arm of the loop counts as a failure too — same leftover. + const store = makeDocStore([staleStored("docs/broken.md")], PRIOR_ETAG); + const { env } = makeDocEnv(); + stubTree(["docs/broken.md"], 'W/"tree-1"', { failContent: true }); + + await pollDocs(REPO, env, store.stub); + + expect(processAndUpsertDocMock).not.toHaveBeenCalled(); + expect(store.etag()).toBe(PRIOR_ETAG); + }); + + it("re-detects the failed doc on the next run, which is what the hold buys", async () => { + const store = makeDocStore([staleStored("docs/broken.md")], PRIOR_ETAG); + const { env } = makeDocEnv(); + processAndUpsertDocMock.mockResolvedValue({ embedded: false, failed: true }); + + stubTree(["docs/broken.md"]); + await pollDocs(REPO, env, store.stub); + + // Next run: the held ETag does not match the tree (this run proved that by + // getting a 200 for it), so the tree comes back 200 and the doc — whose + // `blobSha` the failed embed never advanced — is changed again. + const second = stubTree(["docs/broken.md"]); + processAndUpsertDocMock.mockClear(); + processAndUpsertDocMock.mockResolvedValue({ embedded: true, failed: false }); + await pollDocs(REPO, env, store.stub); + + expect(second.conditionalHits).toEqual([PRIOR_ETAG]); + expect(second.contentFetches).toEqual(["docs/broken.md"]); + expect(processAndUpsertDocMock).toHaveBeenCalledTimes(1); + // Nothing left behind now, so the ETag is finally allowed to advance. + expect(store.etag()).toBe('W/"tree-1"'); + }); + + it("advances the ETag when every changed doc embedded cleanly", async () => { + const store = makeDocStore([staleStored("docs/ok.md")], PRIOR_ETAG); + const { env } = makeDocEnv(); + const { contentFetches } = stubTree(["docs/ok.md"]); + + await pollDocs(REPO, env, store.stub); + + expect(contentFetches).toEqual(["docs/ok.md"]); + expect(store.etag()).toBe('W/"tree-1"'); + }); +}); diff --git a/src/poller.test.ts b/src/poller.test.ts index e0a3d97..ca7322b 100644 --- a/src/poller.test.ts +++ b/src/poller.test.ts @@ -10,10 +10,12 @@ const { fetchCommitDetailMock, processAndUpsertCommitDiffMock, processAndUpsertIssueMock, + processAndUpsertReleaseMock, } = vi.hoisted(() => ({ fetchCommitDetailMock: vi.fn(), processAndUpsertCommitDiffMock: vi.fn(), processAndUpsertIssueMock: vi.fn(), + processAndUpsertReleaseMock: vi.fn(), })); vi.mock("./pipeline.js", async (importOriginal) => { @@ -23,15 +25,20 @@ vi.mock("./pipeline.js", async (importOriginal) => { fetchCommitDetail: fetchCommitDetailMock, processAndUpsertCommitDiff: processAndUpsertCommitDiffMock, processAndUpsertIssue: processAndUpsertIssueMock, + processAndUpsertRelease: processAndUpsertReleaseMock, }; }); const { pollDiffs, pollRepo, + pollReleases, nextForwardDiffWatermark, nextBackfillDiffWatermark, nextIssueWatermark, + MAX_EMBEDDINGS_PER_RUN, + MAX_RELEASE_UPSERTS_PER_REPO_PER_RUN, + MAX_DOC_FETCHES_PER_REPO_PER_RUN, } = await import("./poller.js"); const REPO = "acme/widgets"; @@ -51,11 +58,16 @@ const commit = (sha: string, date: string): FakeCommit => ({ sha, date }); * `/watermark` GET + POST surface the diff poller uses, backed by a Map the * test can read and seed. */ -function makeStore(seed: Record = {}) { +function makeStore( + seed: Record = {}, + etagSeed: Record = {}, +) { const watermarks = new Map(Object.entries(seed)); - const etags = new Map(); + const etags = new Map(Object.entries(etagSeed)); /** Records written through `/upsert` (the empty-hash retry markers). */ const upserts: Array<{ number: number; bodyHash: string }> = []; + /** Records written through `/upsert-release` (same retry markers, release side). */ + const releaseUpserts: Array<{ tagName: string; bodyHash: string }> = []; const stub = { async fetch(request: Request): Promise { const url = new URL(request.url); @@ -83,10 +95,21 @@ function makeStore(seed: Record = {}) { upserts.push({ number: body.number, bodyHash: body.bodyHash }); return new Response("ok"); } + if (request.method === "POST" && url.pathname === "/upsert-release") { + const body = (await request.json()) as { tagName: string; bodyHash: string }; + releaseUpserts.push({ tagName: body.tagName, bodyHash: body.bodyHash }); + return new Response("ok"); + } return new Response("ok"); }, }; - return { stub: stub as unknown as DurableObjectStub, watermarks, etags, upserts }; + return { + stub: stub as unknown as DurableObjectStub, + watermarks, + etags, + upserts, + releaseUpserts, + }; } /** @@ -190,6 +213,54 @@ function stubIssueList(issues: ReturnType[]) { return { sinceQueries }; } +/** One release as the GitHub list endpoint returns it (subset the poller reads). */ +function fakeRelease(tag: string) { + return { + tag_name: tag, + name: `Release ${tag}`, + body: "notes", + prerelease: false, + created_at: "2026-07-01T00:00:00.000Z", + published_at: "2026-07-01T00:00:00.000Z", + html_url: `https://github.com/${REPO}/releases/tag/${tag}`, + }; +} + +const RELEASE_ETAG = 'W/"releases-v2"'; + +/** + * Stub the global fetch with a fake GitHub releases endpoint. + * + * The conditional-request contract is the load-bearing part: echoing back the + * ETag the poller stored answers 304, which is what turns "stored an ETag over + * unfinished work" into a stall rather than a wasted request. + */ +function stubReleaseList(releases: ReturnType[]) { + const conditionalHits: string[] = []; + + const fetchMock = vi.fn(async (input: string | URL, init?: RequestInit) => { + const headers = (init?.headers ?? {}) as Record; + const sent = headers["If-None-Match"]; + if (sent) conditionalHits.push(sent); + if (sent === RELEASE_ETAG) { + return new Response(null, { status: 304 }); + } + return new Response(JSON.stringify(releases), { + status: 200, + headers: { "Content-Type": "application/json", etag: RELEASE_ETAG }, + }); + }); + + vi.stubGlobal("fetch", fetchMock); + return { conditionalHits }; +} + +/** Tags handed to the embed pipeline, i.e. the releases a run attempted. */ +const attemptedReleases = (): string[] => + processAndUpsertReleaseMock.mock.calls.map((call) => + String((call[3] as { tag_name: string }).tag_name), + ); + /** Issue numbers handed to the embed pipeline, i.e. the items a run attempted. */ const attemptedIssues = (): number[] => processAndUpsertIssueMock.mock.calls.map((call) => Number((call[3] as { number: number }).number)); @@ -204,6 +275,7 @@ beforeEach(() => { fetchCommitDetailMock.mockReset(); processAndUpsertCommitDiffMock.mockReset(); processAndUpsertIssueMock.mockReset(); + processAndUpsertReleaseMock.mockReset(); // Default: every issue embeds cleanly. processAndUpsertIssueMock.mockResolvedValue({ embedded: true, @@ -211,6 +283,13 @@ beforeEach(() => { metadataUpdated: false, failed: false, }); + // Default: every release embeds cleanly. + processAndUpsertReleaseMock.mockResolvedValue({ + embedded: true, + skippedUnchanged: false, + metadataUpdated: false, + failed: false, + }); // Default: every commit ingests cleanly. fetchCommitDetailMock.mockImplementation(async (_repo: string, sha: string) => ({ sha, @@ -572,6 +651,123 @@ describe("poller: pollRepo watermark / retry boundary", () => { }); }); +describe("poller: the fan-out caps that stand in for an embedding guard", () => { + // `pollReleases` and `pollDocs` carry no embedding cap of their own. Issue #211 + // removed the branches that used to hold one, because each was unreachable: the + // loop's embed count can never exceed its fan-out count, and the fan-out cap + // fires first at a value below MAX_EMBEDDINGS_PER_RUN. That relation between + // constants is the whole reason those loops are safe without a guard, and it is + // the kind of thing a later cap adjustment breaks silently — this repo has + // retuned fan-out caps more than once (issue #134 took the comment fetch cap + // from 30 to 10). So the relation is asserted here rather than left to the + // comments at the cap sites. + // + // If either assertion below fails, raising the cap was not by itself wrong — + // but the loop it belongs to now needs its embedding guard back, folded into + // that loop's ETag-hold condition (`leftWorkBehind` in `pollReleases`, + // `holdEtag` in `pollDocs`) so a deferred item still holds the ETag. + it("keeps both fan-out caps below the embedding budget", () => { + expect(MAX_RELEASE_UPSERTS_PER_REPO_PER_RUN).toBeLessThan(MAX_EMBEDDINGS_PER_RUN); + expect(MAX_DOC_FETCHES_PER_REPO_PER_RUN).toBeLessThan(MAX_EMBEDDINGS_PER_RUN); + }); +}); + +describe("poller: pollReleases ETag hold", () => { + // Before issue #211 the ETag write looked at the upsert cap alone. An embed + // failure marks the release with an empty `bodyHash` for retry exactly the way + // the cap does, but the run still stored the fresh ETag — so the next cron was + // answered 304 and returned before it reached the marked release, which then + // waited for some *other* release to change. + const KEY = `releases:${REPO}`; + const PRIOR_ETAG = 'W/"releases-v1"'; + const seeded = () => + makeStore({ [KEY]: "2026-07-01T00:00:00.000Z" }, { [KEY]: PRIOR_ETAG }); + + it("holds the prior ETag when a release failed to embed", async () => { + const { stub, etags } = seeded(); + stubReleaseList([fakeRelease("v1.0.0"), fakeRelease("v1.1.0")]); + + processAndUpsertReleaseMock.mockImplementation( + async (_env: unknown, _stub: unknown, _repo: string, release: { tag_name: string }) => + release.tag_name === "v1.1.0" + ? { embedded: false, skippedUnchanged: false, metadataUpdated: false, failed: true } + : { embedded: true, skippedUnchanged: false, metadataUpdated: false, failed: false }, + ); + + await pollReleases(REPO, env, stub); + + expect(attemptedReleases()).toEqual(["v1.0.0", "v1.1.0"]); + expect(etags.get(KEY)).toBe(PRIOR_ETAG); + }); + + it("reprocesses the failed release on the next run, which is what the hold buys", async () => { + const { stub } = seeded(); + stubReleaseList([fakeRelease("v1.0.0")]); + processAndUpsertReleaseMock.mockResolvedValue({ + embedded: false, + skippedUnchanged: false, + metadataUpdated: false, + failed: true, + }); + + await pollReleases(REPO, env, stub); + + // Next run: the held ETag does not match the current list (this run proved + // that by getting a 200 for it), so the list comes back 200 and the release + // is attempted again. + const { conditionalHits } = stubReleaseList([fakeRelease("v1.0.0")]); + processAndUpsertReleaseMock.mockClear(); + processAndUpsertReleaseMock.mockResolvedValue({ + embedded: true, + skippedUnchanged: false, + metadataUpdated: false, + failed: false, + }); + await pollReleases(REPO, env, stub); + + expect(conditionalHits).toEqual([PRIOR_ETAG]); + expect(attemptedReleases()).toEqual(["v1.0.0"]); + }); + + it("stores the fresh ETag when every release landed", async () => { + const { stub, etags } = seeded(); + stubReleaseList([fakeRelease("v1.0.0")]); + + await pollReleases(REPO, env, stub); + + expect(etags.get(KEY)).toBe(RELEASE_ETAG); + }); + + it("still holds the ETag when the upsert cap deferred releases", async () => { + // The issue #149 guard, kept: 12 releases against a per-run upsert cap of 10. + const releases = Array.from({ length: 12 }, (_, i) => fakeRelease(`v1.0.${i}`)); + const { stub, etags, releaseUpserts } = seeded(); + stubReleaseList(releases); + + await pollReleases(REPO, env, stub); + + expect(attemptedReleases()).toHaveLength(10); + // The two over-cap releases are marked for retry, not embedded. + expect(releaseUpserts.map((r) => r.tagName)).toEqual(["v1.0.10", "v1.0.11"]); + expect(releaseUpserts.every((r) => r.bodyHash === "")).toBe(true); + expect(etags.get(KEY)).toBe(PRIOR_ETAG); + }); + + it("returns early on 304 without touching the pipeline", async () => { + // The stall this hold prevents: with the fresh ETag stored, this is the shape + // every subsequent run would take while a marked release sat unprocessed. + const { stub } = makeStore( + { [KEY]: "2026-07-01T00:00:00.000Z" }, + { [KEY]: RELEASE_ETAG }, + ); + stubReleaseList([fakeRelease("v1.0.0")]); + + await pollReleases(REPO, env, stub); + + expect(attemptedReleases()).toEqual([]); + }); +}); + describe("poller: pollDiffs backfill watermark", () => { it("does not step over a failed commit", async () => { const commits = [ diff --git a/src/poller.ts b/src/poller.ts index c5f38ea..7bb226b 100644 --- a/src/poller.ts +++ b/src/poller.ts @@ -39,12 +39,23 @@ import { deleteEdgesForVector } from "./graph.js"; /** GitHub API page size */ const PER_PAGE = 100; -/** Maximum number of embeddings to generate per single cron run. +/** Maximum number of embeddings the issue / PR poller generates per cron run. * Prevents Workers AI rate-limit errors on large repos. * Remaining issues are stored with empty bodyHash and retried next cron. * The retry only happens because the watermark is held at the first such item - * (see `nextIssueWatermark`); the empty hash marks the item, it does not fetch it. */ -const MAX_EMBEDDINGS_PER_RUN = 50; + * (see `nextIssueWatermark`); the empty hash marks the item, it does not fetch it. + * + * Scope: `pollRepo` only. The releases and docs surfaces are bounded by their + * own fan-out caps (`MAX_RELEASE_UPSERTS_PER_REPO_PER_RUN` / + * `MAX_DOC_FETCHES_PER_REPO_PER_RUN`, both 10), which sit below this number and + * therefore always fire first — so those loops carry no embedding guard of their + * own (issue #211). Raising either cap past this value means reinstating one; + * the note at each cap site says so. Comments and wiki have their own budgets + * (`MAX_COMMENTS_EMBEDDED_PER_REPO` / `MAX_WIKI_EMBEDDINGS_PER_RUN`). + * + * Exported so the relation that keeps those two loops guard-free is asserted by + * a test rather than only stated in comments (issue #211). */ +export const MAX_EMBEDDINGS_PER_RUN = 50; /** Maximum number of API pages to fetch per single cron run. * Prevents Cloudflare Worker CPU time limit on large repos (e.g. 900+ issues initial sync). @@ -106,8 +117,12 @@ const MAX_COMMENT_FETCHES_PER_REPO_PER_RUN = 10; * - Cap = 10. Worst-case 5 × 10 × 5 = 250 subrequests for the docs surface * across all repos, well under the per-surface envelope. * Remaining changed docs are picked up on the next cron run (blob SHA stays - * unchanged in the store until the doc is successfully upserted). */ -const MAX_DOC_FETCHES_PER_REPO_PER_RUN = 10; + * unchanged in the store until the doc is successfully upserted). + * + * Must stay below `MAX_EMBEDDINGS_PER_RUN`: that relation is what makes the + * docs loop's embed count bounded without a guard of its own (issue #211). + * Asserted by a test; see the note at the cap site in `pollDocs`. */ +export const MAX_DOC_FETCHES_PER_REPO_PER_RUN = 10; /** Maximum docs reaped (Vectorize + FTS5 + store row) per repo per cron run. * Mirrors `MAX_WIKI_DELETIONS_PER_REPO_PER_RUN`: each reap fans out to 3 @@ -144,8 +159,12 @@ const MAX_DOC_DELETIONS_PER_REPO_PER_RUN = 5; * `MAX_EMBEDDINGS_PER_RUN=50` × ~5 ≈ 250, the LIGHT_CRON worst case stays * around 750 subrequests, well under the 1000 ceiling. * Remaining releases are stored with empty bodyHash so the next cron run - * retries them (existing pattern in `pollReleases`). */ -const MAX_RELEASE_UPSERTS_PER_REPO_PER_RUN = 10; + * retries them (existing pattern in `pollReleases`). + * + * Must stay below `MAX_EMBEDDINGS_PER_RUN`: that relation is what makes the + * releases loop's embed count bounded without a guard of its own (issue #211). + * Asserted by a test; see the note at the cap site in `pollReleases`. */ +export const MAX_RELEASE_UPSERTS_PER_REPO_PER_RUN = 10; /** Maximum number of commits fetched in the forward (webhook-redundancy) phase * of the diff poller per repo per run. @@ -631,8 +650,14 @@ async function fetchReleases( /** * Poll a single repository for release updates. + * + * Holds the releases ETag back whenever the run leaves work behind — the upsert + * cap deferred releases (issue #149) or an embed failed (issue #211) — so the + * next run is not answered 304 before it can see the retry markers. + * + * Exported for tests; production callers reach it through `handleScheduled`. */ -async function pollReleases( +export async function pollReleases( repo: string, env: Env, storeStub: DurableObjectStub, @@ -720,36 +745,15 @@ async function pollReleases( continue; } - // Enforce per-run embedding limit - if (embedded >= MAX_EMBEDDINGS_PER_RUN) { - if (embedded === MAX_EMBEDDINGS_PER_RUN) { - console.warn( - `Release embedding batch limit reached (${MAX_EMBEDDINGS_PER_RUN}). ` + - `Remaining releases will be retried next cron run.`, - ); - } - // Store record with empty bodyHash to trigger retry on next poll - const name = release.name ?? release.tag_name; - const record: ReleaseRecord = { - repo, - tagName: release.tag_name, - name, - body: release.body ?? "", - prerelease: release.prerelease, - bodyHash: "", - createdAt: release.created_at, - publishedAt: release.published_at ?? release.created_at, - }; - await storeStub.fetch( - new Request("http://store/upsert-release", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(record), - }), - ); - continue; - } - + // No embedding cap of its own on this surface: `embedded` can never exceed + // `upsertsIssued`, and the cap above stops the loop at + // MAX_RELEASE_UPSERTS_PER_REPO_PER_RUN (10), below MAX_EMBEDDINGS_PER_RUN + // (50). A guard here used to exist and was unreachable by that relation; + // issue #211 removed it, because an unreachable branch cannot be exercised by + // a test and left the reader unable to tell whether it also owed the ETag + // hold below. Raise this cap past MAX_EMBEDDINGS_PER_RUN and an embedding + // guard has to come back here, folded into `leftWorkBehind`. Same note in + // `pollDocs`. upsertsIssued++; const result = await processAndUpsertRelease(env, storeStub, repo, release); @@ -762,31 +766,33 @@ async function pollReleases( } } - // Update watermark with ETag — but skip the ETag write when the run hit - // the per-run upsert cap, otherwise the next cron will short-circuit on - // 304 and never reprocess the deferred (empty-bodyHash) releases (issue - // #149). - if (!upsertBudgetExhausted) { - await storeStub.fetch( - new Request("http://store/watermark", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ repo: watermarkKey, lastPolledAt: new Date().toISOString(), etag: responseEtag }), - }), - ); - } else { - await storeStub.fetch( - new Request("http://store/watermark", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ repo: watermarkKey, lastPolledAt: new Date().toISOString(), etag: storedEtag }), + // Update watermark with ETag — but hold the prior ETag whenever the run left + // work behind, on the same condition the issue / PR and docs surfaces use + // (issue #215 / #211): the upsert cap deferred releases (issue #149), or an + // embed failed. Both mark the release with an empty `bodyHash` for retry, and + // both need the next run to actually look. Storing the fresh ETag would make + // that run's conditional request answer 304 and return before it did — the + // marked release would then wait for some *other* release to change. + // Holding the prior ETag is enough to force a 200: this run sent it and got + // 200 back, so the list already differs from it. `lastPolledAt` still advances + // so the run stays observable. + const leftWorkBehind = upsertBudgetExhausted || failed > 0; + await storeStub.fetch( + new Request("http://store/watermark", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + repo: watermarkKey, + lastPolledAt: new Date().toISOString(), + etag: leftWorkBehind ? storedEtag : responseEtag, }), - ); - } + }), + ); console.log( `${repo} releases: ${releases.length} total, ${embedded} embedded, ${skipped} unchanged, ${failed} failed, ` + - `upserts_issued=${upsertsIssued}/${MAX_RELEASE_UPSERTS_PER_REPO_PER_RUN}`, + `upserts_issued=${upsertsIssued}/${MAX_RELEASE_UPSERTS_PER_REPO_PER_RUN}` + + `${leftWorkBehind ? " (ETag held)" : ""}`, ); } @@ -1044,15 +1050,14 @@ export async function pollDocs( break; } - if (embedded >= MAX_EMBEDDINGS_PER_RUN) { - console.warn( - `Doc embedding batch limit reached (${MAX_EMBEDDINGS_PER_RUN}). ` + - `Remaining docs will be retried next cron run.`, - ); - // Stop processing — unchanged blobSha in store means next poll retries - break; - } - + // No embedding cap of its own on this surface: `embedded` can never exceed + // `fetchesIssued`, and the cap above stops the loop at + // MAX_DOC_FETCHES_PER_REPO_PER_RUN (10), below MAX_EMBEDDINGS_PER_RUN (50). + // A guard here used to exist and was unreachable by that relation; issue #211 + // removed it, because an unreachable branch cannot be exercised by a test and + // left the reader unable to tell whether it also owed the ETag hold below. + // Raise this cap past MAX_EMBEDDINGS_PER_RUN and an embedding guard has to + // come back here, folded into `holdEtag`. Same note in `pollReleases`. try { // Fetch file content fetchesIssued++; @@ -1122,14 +1127,18 @@ export async function pollDocs( removedDocs++; } - // Update watermark with ETag — but only when the run completed without - // hitting a per-run cap. If either cap was hit, leftover work remains: - // changed docs still to embed (issue #149) or deleted docs still to reap - // (issue #203). Persisting the new ETag would cause the next cron to - // short-circuit on 304 and never see either. Holding the prior ETag forces a - // fresh tree fetch next run so `changedEntries` and `deletedDocs` both - // repopulate. lastPolledAt is still bumped so observability sees the run. - const holdEtag = fetchBudgetExhausted || deleteBudgetExhausted; + // Update watermark with ETag — but only when the run left no work behind, the + // same condition the issue / PR and releases surfaces use (issue #215 / #211). + // Three ways work is left: changed docs still to embed (fetch cap, issue + // #149), deleted docs still to reap (delete cap, issue #203), or a doc whose + // embed failed (issue #211 — its store row keeps the old `blobSha`, so it is + // still "changed" next run). Persisting the new ETag would cause the next cron + // to short-circuit on 304 and never see any of them. Holding the prior ETag + // forces a fresh tree fetch next run so `changedEntries` and `deletedDocs` + // both repopulate — this run sent that ETag and got 200 back, so the tree + // already differs from it. lastPolledAt is still bumped so observability sees + // the run. + const holdEtag = fetchBudgetExhausted || deleteBudgetExhausted || failed > 0; await storeStub.fetch( new Request("http://store/watermark", { method: "POST", @@ -1145,7 +1154,8 @@ export async function pollDocs( console.log( `${repo} docs: ${docEntries.length} found, ${embedded} embedded, ${skipped} unchanged, ${failed} failed, ` + `${removedDocs}/${deletedDocs.length} deleted, ` + - `fetches_issued=${fetchesIssued}/${MAX_DOC_FETCHES_PER_REPO_PER_RUN}`, + `fetches_issued=${fetchesIssued}/${MAX_DOC_FETCHES_PER_REPO_PER_RUN}` + + `${holdEtag ? " (ETag held)" : ""}`, ); }