fix(webhook): tear down push-path doc delete surfaces independently [webhook, docs, tests] - #207
Conversation
…webhook, docs, tests] The push-path doc delete wrapped one removed file in a single outer try, so a Vectorize failure jumped to the catch and neither the D1 FTS5 row nor the store row was ever touched. The deleted doc kept surfacing in search — the D1 rows are the ones users actually retrieve. Every other delete path in this file already split its surfaces; docs was the last one left, and it is the primary path (cron is its fallback), so it is the one that actually runs. The three surfaces — Vectorize, D1 FTS5, store row — now each carry their own try, adopting the shape the cron reap took in #205. Graph edges are still not torn down, same invariant as there: a doc vector ID is never a `doc_edges` endpoint. No per-run cap, unlike the cron reap: a push payload names its own removals, so this loop is bounded by the event rather than by an accumulated backlog. `deleted` in the response now counts docs whose three surfaces all came down, which makes `removed - deleted` the partial-teardown count. The cron reap counts attempts instead; the reason for the divergence is in the PR body. handlePushEvent is exported for the new test file. issue 本文の「Vectorize 失敗時に FTS5 だけ消えて store 行が残る」は実測と食い 違っていた。内側 try は Vectorize 呼び出しの後ろにあるので、Vectorize が throw した時点で FTS5 も store も一度も触られない。取り残しの範囲は本文の記述より広 かったが、欠陥そのもの（Vectorize の失敗が D1 側の teardown を丸ごと飛ばす）は そのまま成立する。 Closes #206
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
github-rag-mcp | d30b5b7 | Aug 02 2026, 12:30 PM |
liplus-lin-lay
left a comment
liplus-lin-lay
left a comment
There was a problem hiding this comment.
Self-review (auto mode, parent agent)
issue 本文の記述誤りを訂正して受理する
本文は「Vectorize 失敗時に FTS5 だけ消えて store 行が残る」と書いていたが、誤り。deleteFtsRow の内側 try は VECTORIZE.deleteByIds の後ろにあるため、Vectorize が throw した時点で外側 catch に飛び、D1 の 2 面はどちらも触られない。実装者が pre-fix 実装（export のみ追加）に対して測り直し、deleteFtsRow の呼び出し回数が 1 ではなく 0 であることを実測で確認している。
欠陥の所在と修正内容は変わらないが、取り残される範囲は本文の記述より広い。訂正は PR 本文と commit message に入っている。issue #206 の本文はこちらで直す。
これは私が本セッションで 3 度目に踏んだ形（#202 継承前提の無検証再掲 / #203 コードの差からデータ欠陥を推論 / 本件 コードを読んだうえで制御フローを gist で記述）。tally に記録済み。
受け入れ条件
- Vectorize 失敗でも FTS5 行と store 行の teardown が実行される — 満たす。3 面を独立 try に分離。pre-fix 実装に対し 6 本中 3 本が fail することを実測確認済みで、テストが空振りでないことの裏取りがある
- 他の webhook 削除経路に変更が無い — 満たす。変更は doc 削除ループのみ
- Vectorize 失敗時の
deletedの値がテストで固定されている — 満たす（removed: 1, deleted: 0）
deleted counter を cron と揃えなかった判断 — 受理する
追記で「揃えないなら理由を残せ」と要求した点。理由が構造的で妥当なので受理する。
cron 側の removedDocs が試行数を数えるのは、その counter が MAX_DOC_DELETIONS_PER_REPO_PER_RUN と ETag hold を制御する budget counter を兼ねているため。予算を消費するのは試行であって成功ではない。
webhook 側には予算も cap も無い。試行数を数えると deleted は常に removed と一致し、レスポンス body は既に docs: { removed, deleted, failed } の形で removed を持っているので、同じ数を二度書くだけになり、部分失敗が delivery log から消える（console.error にしか残らない）。
counter が両経路で違う仕事をしている以上、値の意味が違うのは構造的な差であって不揃いではない。観測可能性を取る側が正しい。
scope
handlePushEvent の export はテストのためで、#205 の pollDocs export と同じ扱い。docs は cron 側の段落と対にする形で Webhook Receiver 節に追加されており、両経路の差（graph edge を消さない理由 / cap が無い理由 / counter の意味の違い）がすべて書かれている。撤回済みの edge 論拠が混入していないことを確認した。
brake
user repo の PR で Li+ source を触っていないため brake 1 / brake 2 の対象外。
次のステップ
auto mode につき human gate なし。self-review pass → merge。
1 participant
Closes #206
何が壊れていたか
src/webhook.tsの doc 削除ループは、1 件を 1 本の外側 try で包んでいた。Vectorize のdeleteByIdsが throw すると、そのまま catch に飛ぶので FTS5 行の削除も store 行の DELETE も一度も実行されない。削除したはずの doc が search に出続ける。実際に retrieval されるのは D1 側の行なので、影響が出るのはここ。同ファイルの他の削除経路（issue/PR、release、comment、review、review comment）は既に surface ごとに try が分かれていた。doc 経路だけが取り残されていて、しかも push は一次経路で cron はその fallback なので、実際に多く通るのはこちら。
issue 本文の記述の訂正
issue 本文は「Vectorize 失敗時に FTS5 だけ消えて store 行が残る」としていたが、実測はそうではなかった。内側の FTS5 try は Vectorize 呼び出しの後ろにあるので、Vectorize が throw した時点で FTS5 にも store にも到達しない。取り残しの範囲は本文の記述より広い。欠陥そのもの（Vectorize の失敗が D1 側の teardown を丸ごと飛ばす）は成立するので、修正内容は変えていない。
pre-fix の実装に対して新テストを走らせて確認した結果:
deleteFtsRowの呼び出し回数が 0（本文の記述どおりなら 1 のはず）変更
3 面（Vectorize / D1 FTS5 / store 行）をそれぞれ独立した try で teardown する形に変えた。#205 で cron 側（
src/poller.ts）が取った形をそのまま持ってきている。doc_edgesの端点は src / dst とも wiki vector ID で（indexWikiEdgesが唯一の writer）、doc vector ID は端点になりえない。0 行にしかマッチしない DELETE を将来仮定で足さない。不変条件が変わったら足すこと、をコメントに残した。handlePushEventを export した（fix(docs): tear down doc reap surfaces independently and cap it per run [poller, docs, tests] #205 でpollDocsを export したのと同じ扱い）。deletedカウンタの意味 — cron に揃えなかった理由issue のコメントで「cron 側（試行数を数える）に揃えるのが素直、揃えないなら理由を PR 本文に」とあったので、揃えなかった理由を書く。
採った意味:
deleted= 3 面すべての teardown が成功した doc の件数。 したがってremoved - deletedが部分失敗の件数になる。cron 側の
removedDocsが試行数を数えているのは、あれがMAX_DOC_DELETIONS_PER_REPO_PER_RUNの cap を駆動し、さらに ETag hold を決める budget counter を兼ねているから。予算を消費するのは試行であって成功ではないので、あちらでは試行数が正しい。webhook 側には budget が無い。ここで試行数を数えると、cap が無い以上ループは
removed全件を回るのでdeletedは常にremoved.sizeと一致する。removedは同じレスポンス body にすでに入っているので、それは同じ数字を 2 回書いているだけになる。しかも部分失敗が body から完全に消え、console.errorを wrangler tail で拾いに行かないと分からなくなる。webhook のレスポンスは GitHub の delivery log に残る、運用者が最も安く見られる観測面なので、そこに信号を残す側を採った。カウンタの職掌が両者で違う（budget counter か、観測用フィールドか）ので、意味が分かれるのはむしろ構造どおりだと判断している。
テスト
src/webhook-push-docs.test.tsを新設（6 ケース）。deleteFtsRowを mock、Vectorize と Store DO は in-memory の stand-in。commit にidを持たせないことで diff indexing 分岐を short-circuit させ、global fetch を stub せずに済ませている（HTTP を打ちに行く変更が入ったら落ちる）。deleted: 1）deleted: 0も固定）removed: 2, deleted: 1）pre-fix 実装に対する検証: export だけ足して修正前のループに戻し、同じテストを走らせて 3 ケースが落ちることを確認済み（vacuous test でないことの確認）。
docs
docs/0-requirements.md/.ja.mdの Webhook Receiver 節に、push 経路の doc 削除の 3 面 teardown・cap を持たない理由・deletedの意味を追記した。cron 側の記述（#205 で追加）と対になる位置。検証
npx tsc --noEmitパスnpx vitest run— 12 files / 131 tests パスnpx vitest run --config vitest.workers.config.ts— 4 files / 53 tests パス