fix(pipeline): batch-embed diffs by token budget, not file count [pipeline, docs, tests] - #237
Conversation
…eline, docs, tests] The commit-diff batch was cut on a file count (MAX_EMBEDDING_BATCH_SIZE = 20), which bounds a Workers AI call only where every patch is assumed to be small. Characters per token vary by an order of magnitude across what this pipeline embeds — roughly 3 for ASCII source, roughly 1 for CJK prose — so 20 inputs truncated at 8000 chars could present many times bge-m3's documented 8192-token per-input maximum in a single call. A commit that broke the assumption failed its whole chunk, and a commit whose vectors never landed is one the diff watermark holds on (#178), so the surface stalled there permanently instead of passing it by. - src/pipeline/embedding.ts: estimateEmbeddingTokens (non-ASCII code units at one token each, ASCII at three chars per token — both the conservative side) and planEmbeddingBatches, which returns index ranges so the caller can cut its own parallel arrays on the same boundary. MAX_EMBEDDING_BATCH_TOKENS replaces MAX_EMBEDDING_BATCH_SIZE, anchored to the model's per-input maximum: a batch has to carry one maximal input, so the budget cannot sit below it, and holding it exactly there keeps one call inside one model context worth of text. MAX_VECTORIZE_UPSERT_BATCH_SIZE goes with it — it was never read, and its whole definition was mirroring the constant that left. - src/pipeline/embed-diff.ts: inputs are built once for the commit and the loop walks the planned ranges. - src/pipeline/embedding.test.ts, src/pipeline/embed-diff.test.ts: the planner contract (coverage, order, no empty range, an over-budget input alone rather than a stall) and the pipeline behaviour, including that each embed call stays paired with its own slice of files. An input whose own estimate exceeds the budget is still sent alone. Cutting it down further belongs to the truncation axis (MAX_EMBEDDING_INPUT_CHARS), and dropping it would lose the file from the index. 件数軸を残さずに置き換えたのは、残せば「どの patch も安い側である」という同じ 仮定が別の場所に残るため。1 batch あたりの call 数は増えうるが、上限は file 数 であり、その file は既に FTS mirror と Store DO の書き込みで subrequest を 消費しているので、増分は fan-out の中で支配的にならない。 Closes #236 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
github-rag-mcp | e236ed3 | Aug 14 2026, 06:14 AM |
review finding (parent,
|
| budget | 1 batch あたり input | batch 数 | batch 軸の subrequest |
|---|---|---|---|
| 8,192 | 約 2 | 約 22 | 約 44 |
| 50,000 前後 | 約 12-16 | 約 3-4 | 約 6-8 |
後者でも 60,000 の天井には当たらない。8192 が買っている安全性は、50,000 前後との比較では存在しない。
issue の constraint との照合
issue 本文の constraint に「通常サイズの commit は従来どおり 1 call にまとめる。call 数を無用に増やさない」がある。20 file × 8000 文字の ASCII commit は約 53,340 token で、8192 予算では 7 batch に割れる。従来 1 call だったものが 7 call になるので、この constraint を満たしていない。
（issue 本文は起票時に壊れていた。実装後に復元済みなので、この constraint は実装時には読めなかったはずである。指摘であって責任追及ではない。）
求める対応
MAX_EMBEDDING_BATCH_TOKENS を、observed な batch 上限 60,000 から安全マージンを引いた値に置き直す。推定器が近似であることを踏まえたマージンは必要なので、60,000 そのものは使わない。値の根拠は既存コメントと同じ水準で残すこと。
anchor を変えない判断をするなら、その理由を commit body に記録してほしい。採用・不採用いずれでも理由を残す形で構わない。
…iling [pipeline, docs, tests] ACCEPT — parent self-review finding on MAX_EMBEDDING_BATCH_TOKENS = 8192. The finding is right and the reason is that 8192 was the wrong axis, not merely a conservative value. It is bge-m3's documented *per-input* maximum. What the endpoint enforces on a batch is an aggregate across every input in the call, and it names the sum it rejected: 3030: Max context reached 85920 tokens but model supports only 60000 The restored issue body carries that log line and the observed sums (60,678 / 64,413 / 74,980 / 85,920). At implementation time the body was the two characters `@-`, so the ceiling was picked from the only number the docs publish. With the real ceiling in hand the anchor does not survive: it is not a margin against 60,000, it is a different quantity that happens to be smaller. - src/pipeline/embedding.ts: WORKERS_AI_BATCH_CONTEXT_LIMIT = 60000 records the ceiling from the error (unpublished, so the error is the source), and MAX_EMBEDDING_BATCH_TOKENS becomes half of it. Naming the ceiling is what keeps the budget readable as a margin rather than as an arbitrary number. - src/pipeline/embedding.test.ts: the budget-to-ceiling relation is asserted, with a floor assertion under it. Pressure on this number runs one way — every batch costs two subrequests on an axis this worker already overruns — so the drift to guard is the budget walking up toward 60,000. - src/pipeline/embed-diff.test.ts: the split case moves from 6 files to 20, which is exactly one chunk under the retired count cap and the shape the endpoint rejected in production. - docs/0-requirements.md + .ja.md: the per-input anchor is replaced by the aggregate ceiling and the margin's reasoning. Half rather than the ~50,000 the finding sketched. The estimator approximates, and only one error direction matters: an estimate landing under the true count puts the call over the ceiling, which fails the chunk and re-enters the stall this PR exists to end. Punctuation-dense payloads (lockfile integrity hashes, minified sources) are where the 3-chars-per-token ASCII ratio runs optimistic, and 2x covers that class. Against the finding's own subrequest arithmetic the difference is small — its 44-file shape plans to 4 batches at 30,000 against the 3-4 it projected for ~50,000 — so the margin is bought for roughly nothing on the axis it was weighed against. Measured across shapes (batches at 30,000 / at the old 8192): 20 files x 1500 ASCII chars (ordinary) 1 / 2 20 files x 8000 ASCII chars (maximal) 2 / 7 44 files x 8000 ASCII chars (observed) 4 / 15 20 files x 8000 CJK chars (worst case) 7 / 20 The finding's constraint check — ordinary commits stay one call — now holds. Its 20x8000 ASCII example lands at 2 rather than 1, and no budget at or under the ceiling makes that one call: the shape estimates to 53,340 tokens, so it exceeds even the ~50,000 the finding sketched. It is the maximal case, not an ordinary one. 指摘は subrequest 軸を根拠に「8192 は買っていない安全性のために隣の逼迫軸を削って いる」と述べていて、その読みが正しい。天井の半分に置くことで、推定器の誤差に対する 実質的な余裕を残したまま、通常 commit は 1 call に戻る。 Refs #236 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
smileygames
left a comment
smileygames
left a comment
There was a problem hiding this comment.
AI self-review (auto mode, parent)
受け入れ条件
| criterion | 結果 |
|---|---|
| 1 batch の推定 token 数が上限（安全マージン込み）を超えない | pass — MAX_EMBEDDING_BATCH_TOKENS = WORKERS_AI_BATCH_CONTEXT_LIMIT / 2 = 30,000。天井 60,000 は error 実測から命名。budget < ceiling と budget > per-input 上限の両方を test が assert している |
| 既存テストが通り、上限超過ケースの回帰テストが追加されている | pass — CI / test / Workers Builds いずれも green。embedding.test.ts に推定器と分割器の契約 15 件、embed-diff.test.ts の分割ケースは 20 file（production が拒否した形）へ |
| 2 repo の forward watermark が boundary commit を越えて前進する | post-merge 観測へ繰り越し — deploy 後の cron tick が必要で、merge 前には閉じられない。gate ではなく観測項目として扱う |
レビュー指摘の帰結
MAX_EMBEDDING_BATCH_TOKENS = 8192 への指摘は ACCEPT された。commit e236ed3 の body に理由が記録されている。
指摘側が「過度に保守的」と書いたところを、実装側は「軸が違う」と読み替えて返している。8192 は bge-m3 の per-input 上限であって、batch 集約の天井 60,000 に対する margin ではない、という整理。この読み替えは正しい。天井を WORKERS_AI_BATCH_CONTEXT_LIMIT として名前で残し、budget をその 1/2 と定義したことで、値が margin として読める形になっている。
値は指摘が素描した ~50,000 ではなく 30,000 が採られた。推定器の誤差方向が非対称（過小評価だけが天井超過を生み、chunk 全体の失敗と stall 再突入に直結する）という根拠で、subrequest 軸への影響は 44 file 形で 4 batch 対 3-4 batch とほぼ差がない。指摘の意図（隣接軸を不要に削らない）は満たされているので、値の相違は受け入れる。
scope 逸脱
issue 本文の literal を超えた変更が 2 点。いずれも受け入れる。
MAX_VECTORIZE_UPSERT_BATCH_SIZEの削除。定義が退役対象の定数のミラーだけで構成されており、reader が tree 内に存在しない。退役と同時でなければ宙に浮く- repo label
done→review-pendingのリネーム。Li+ 現行仕様との語彙ずれの解消。description は元からreview-pendingの意味そのもので、既存 2 件の issue は rename により保持されている
next step
auto mode のため human check なし。self-review pass をもって AI が直接 merge する。merge 後、cron 1 tick 分の watermark 前進観測を実施する。
subagent が 2 回にわたり報告した per-commit subrequest 上限の件（diff path に per-commit file cap が無く、300 file の commit が単独で 1000-subrequest 予算に迫りうる。#178 不変条件により同じ形で stall する）は、この issue の literal 外のため別 issue を起票する。
1 participant
Closes #236
commit diff の batch embed の分割軸を、file 件数（
MAX_EMBEDDING_BATCH_SIZE= 20）から推定 token 予算（MAX_EMBEDDING_BATCH_TOKENS= 8192）へ置き換える。件数は「どの patch も安い側である」と仮定したときにしか call を縛れず、その仮定を破る commit は chunk 全体を失敗させ、issue #178 の watermark 不変条件により diff surface がその commit で恒久停止していた。単独で予算を超える input は落とさず単独で送る。call 数の上限は file 数であり、その file は既に FTS mirror と Store DO 書き込みで subrequest を消費しているので、増分は fan-out の中で支配的にならない。要件記述（
docs/0-requirements.md/.ja.md）を同一 PR で更新済み。起票時に@-の 2 文字だけで保存されていた issue 本文も、title と実装から再構成して書き直した。Release type: patch