Skip to content

fix(poller): hold the ETag on any leftover, not just a cap hit [poller, docs, tests] - #218

Merged
liplus-lin-lay merged 2 commits into
mainfrom
211-etag-hold-on-embed-failure
Aug 3, 2026
Merged

fix(poller): hold the ETag on any leftover, not just a cap hit [poller, docs, tests]#218
liplus-lin-lay merged 2 commits into
mainfrom
211-etag-hold-on-embed-failure

Conversation

@liplus-lin-lay

@liplus-lin-lay liplus-lin-lay commented Aug 3, 2026

Copy link
Copy Markdown
Member

Closes #211

何を直したか

release 面と docs 面の ETag 保存判定が、「やり残しの理由」の一部しか見ていなかった。#215 で issue / PR 面に入った形(やり残しがあるどの理由でも ETag を保存しない)に 2 面を揃えた。

修正前 修正後
issue / PR capped || retryBoundary !== undefined 変更なし(#215 で既に一般形)
release upsertBudgetExhausted のみ upsertBudgetExhausted || failed > 0
docs fetchBudgetExhausted || deleteBudgetExhausted + || failed > 0

embed に失敗した item には retry の印が付く(release は空の bodyHash、doc は store 側で進まない blobSha)。しかし fresh ETag を保存してしまうと次 cron が 304 で早期 return し、印を見る機会そのものが来ない。印の付いた item は、同じ面の無関係な item が変更されるまで待たされていた。

据え置き方式は既存のまま(直前の ETag を保持)。この run は保存済み ETag を送って 200 を得ている=その面は既にその ETag と異なるので、次 run も 200 になる。issue 面の「空にする」と等価。

到達不能だった embedding cap 分岐

両面で削除した。embeddedupsertsIssued / fetchesIssued を超えられず、手前の fan-out cap が 10(MAX_EMBEDDINGS_PER_RUN = 50 より小さい)で止めるため、構造的に届かなかった分岐である。

削除を選んだ理由は 2 つ。到達不能な分岐は test で叩けない。そして「この分岐も ETag の据え置きを負うべきか」を読み手が判断できない状態を残す——これが今回の穴が欠陥ではなく曖昧さとして読めてしまった原因でもある。代わりに各 cap の位置に定数関係をコメントで明示し、fan-out cap を 50 より上に引き上げる場合は embedding guard を復活させる必要があると読める形にした。両面で同じ判断・同じ文面。

self-review 差し戻し対応(2 commit 目)

guard 削除の安全性が 3 箇所のコメントだけに支えられていて、2 つの定数関係を破っても落ちるテストが無かった点を修正した。

  • MAX_EMBEDDINGS_PER_RUN / MAX_RELEASE_UPSERTS_PER_REPO_PER_RUN / MAX_DOC_FETCHES_PER_REPO_PER_RUN を export
  • keeps both fan-out caps below the embedding budget で 2 つの関係を検証(fix(backfill): hold the index cursor at the first uningested candidate [backfill, docs, tests] #217MAX_INDEX_BACKFILL_LIMIT と同じ扱い)
  • テストのコメントに「落ちた時に何をすべきか」を明記: 新しい cap が誤りという判定ではなく、その loop に embedding guard を戻し、その loop の ETag 据え置き条件(leftWorkBehind / holdEtag)に組み込む必要がある、という意味
  • 各定数の docstring にも「テストで検証している」旨を追記
  • docs の該当段落をコメント依拠からテスト依拠に書き換え

cap を 100 / 60 に上げて assertion が実際に落ちることを両方向で確認済み。

テスト

pollReleases を test 用に export(pollDocs と同じ扱い)。

  • src/poller.test.ts — release 面 5 本: embed 失敗で直前 ETag 据え置き / 次 run で再処理される(据え置きが買っているもの)/ 全件成功なら fresh ETag 保存 / upsert cap の bug(poller): pollDocs / pollReleases exhaust Worker subrequest budget #149 guard は健在 / 304 早期 return の形
  • src/poller-docs.test.ts — docs 面 4 本: embed 失敗で据え置き / content fetch 例外(catch 経路)でも据え置き / 次 run で再検出 / 全件成功なら fresh ETag

修正を戻すと release 2 本・docs 3 本が落ちることを確認済み(回帰テストとして機能している)。

全体: vitest run 208 passed / workers pool 58 passed / tsc --noEmit clean。

実測について

issue 記載どおり実測はしていない。再現には embed 失敗を意図的に起こす必要があり本番で起こす筋ではないため、検証は上記の回帰テストで行っている。

docs

docs/0-requirements.md / .ja.md に「ETag の据え置き」を 3 面共通の不変条件として一箇所に書き、各面の記述はそこを参照する形にした。監査結果(comment 面は条件付き request を出さない / wiki の etag 列は walk cursor で HTTP ETag ではない)も、書き漏らしではなく構造上の理由として明記してある。

🤖 Generated with Claude Code

…r, docs, tests]

The release and docs pollers decided their ETag write from a subset of the
reasons a run can leave work behind. `pollReleases` looked at
`upsertBudgetExhausted` alone and `pollDocs` at its two caps, so on both an
embed failure stored the fresh ETag: the failed item carried its retry marker
(empty `bodyHash` / unadvanced `blobSha`) but the next run was answered 304 and
returned before it could look, leaving the item to wait for an unrelated item
on the same surface to change. Both now use the shape issue #215 gave the
issue / PR surface — hold whenever anything was left behind.

The unreachable embedding-cap branches are removed on both surfaces, with the
constant relation that made them unreachable (fan-out cap 10 < 50) named at
each cap site instead. Same decision on both, per the issue: an unreachable
branch cannot be exercised by a test, and it left the reader unable to tell
whether it also owed the ETag hold.

3 面のうち issue / PR 面だけが #215 で正しい形になっており、release 面と docs
面が取り残されていた。ETag 保存の判定を「やり残しがあるか」の一条件に揃え、
面ごとに別の条件を持たせない形にした。到達不能だった embedding cap 分岐は
両面で削除し、代わりに定数関係をコメントで明示して、次に定数を触る者が関係に
気づける形にしてある。

Closes #211
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 3, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
github-rag-mcp 4804d73 Aug 03 2026, 08:01 AM

… [poller, docs, tests]

The guard-free release and docs loops rest on two constant relations
(MAX_RELEASE_UPSERTS_PER_REPO_PER_RUN and MAX_DOC_FETCHES_PER_REPO_PER_RUN each
below MAX_EMBEDDINGS_PER_RUN). Until now only comments at the three cap sites
kept them, and nothing failed if one was broken — the procedure form of a rule
that wants to be a structure. Fan-out caps on this repo do get retuned (issue
#134 took the comment fetch cap from 30 to 10), so raising one past 50 would
have removed that surface's embed bound in silence.

The three constants are exported and the relation is asserted, the same
treatment MAX_INDEX_BACKFILL_LIMIT got in issue #217. The test names what a
failure means: not that the new cap is wrong, but that the loop needs its
embedding guard back, folded into that loop's ETag-hold condition.

guard 削除の安全性が 3 箇所のコメントだけに支えられていて、関係を破っても落ちる
テストが無かった。定数を export して関係を検証し、落ちた時に「削除した guard を
戻す必要がある」と読めるメッセージにしてある。docs の該当箇所もコメント依拠から
テスト依拠に書き換えた。

Refs #211

@liplus-lin-lay liplus-lin-lay left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI self-review (auto mode) — 差し戻し対応後

受入基準の充足

issue #211 の修正方針 2 点を満たしている。

方針 結果
1. 両面の ETag 保存判定を「やり残しがあるか」で決め、3 面で形を揃える 充足。release = upsertBudgetExhausted || failed > 0、docs = fetchBudgetExhausted || deleteBudgetExhausted || failed > 0。issue / PR 面(#215)と同じ一般形
2. 到達不能な embedding cap 分岐を両面で同じ判断で決着させ、理由を残す 充足。両面で削除し、ほぼ同一文面のコメントを両 cap サイトに置いた

CI green(test / CI / Workers Builds)。docs は同一 PR 内(0-requirements 両言語)。

diff 実読で確認した点

  • 削除した分岐が本当に到達不能だったこと: release は upsertsIssued++processAndUpsertRelease の直前、embedded++ はその成功時のみ。したがって embedded <= upsertsIssued <= 10 < 50。docs も fetchesIssued++ → 成功時 embedded++ で同じ関係。コメントの主張を実際の制御フロー上で確認した
  • 直前の ETag を保持する方式の妥当性: 「この run は保持中の ETag を送って 200 が返っている=一覧 / tree はすでにそれと異なる」ため次 run も 200 になる。issue / PR 面は空にする方式で、どちらも機能する。issue 本文の指示どおり方式は変更していない
  • docs 面の failed が content fetch 例外も拾うこと: try の外側 catch でも failed++ される経路があり、テストが両方を見張っている
  • 観測性: 両面のログに (ETag held) が入り、据え置きが起きた run が読める

差し戻し 1 点の対応

所見: guard 削除の安全性が 2 つの定数関係にのみ依存し、それを守るのがコメント 3 箇所だけで、破っても落ちるテストが無かった。rules/model/subtractive-structural-beauty.md の「将来の実行が保証されない手順は確実に実行される構造へ置き換える」に該当。

対応を確認した(commit 4804d73):

  • 定数 3 本を export し、MAX_RELEASE_UPSERTS_PER_REPO_PER_RUN < MAX_EMBEDDINGS_PER_RUNMAX_DOC_FETCHES_PER_REPO_PER_RUN < MAX_EMBEDDINGS_PER_RUN を検証するテストを追加。#217 の envelope テストと同じ型
  • 失敗時のメッセージが 2 段の要求として書かれている——「cap 引き上げ自体が誤りではない。guard を戻し、かつその loop の ETag 据え置き条件(leftWorkBehind / holdEtag)に組み込む必要がある」。後段が入っている点が要点で、guard を戻すだけで据え置き条件に組み込まなければ、今回の穴が別の形で再発する。指示した要件より一段踏み込んでいる
  • docs の該当段落を「コメントで明示している」から「テストで検証している」へ書き換え。差し戻し前の記述が実装と食い違うため、同じ 1 点の範囲内として妥当
  • cap を 100 / 60 に上げて両方向で assertion が落ちることを実測確認済み

scope の逸脱

無し。差し戻しの範囲は指示した 1 点に収まっている。

次のステップ

auto mode のため human gate 無し。self-review pass → merge。deploy 後の運用作業は無い——変更の効果は次に embed 失敗が起きた時に現れる。実測は未実施で、検証は回帰テストによる(issue 本文の方針どおり。本番で embed 失敗を意図的に起こす筋のものではない)。

@liplus-lin-lay
liplus-lin-lay merged commit 0eb29f7 into main Aug 3, 2026
3 checks passed
@liplus-lin-lay
liplus-lin-lay deleted the 211-etag-hold-on-embed-failure branch August 3, 2026 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Release and docs pollers refresh their ETag even when an embed failure left work behind

1 participant