perf(retain): rebuild a chunk run from the body instead of guessing its separators (#3790) - #3808
Draft
nicoloboschi wants to merge 1 commit into
Draft
perf(retain): rebuild a chunk run from the body instead of guessing its separators (#3790)#3808nicoloboschi wants to merge 1 commit into
nicoloboschi wants to merge 1 commit into
Conversation
…ts separators (#3790) The retain splitter packs an oversized item's native chunks into runs worth retain_batch_tokens, then rebuilt each run's text by guessing what sat between the chunks — a merged JSON array, "\n\n".join, "\n".join — accepting a guess only when re-chunking it reproduced the run exactly. When none did, the run degraded to one sub-batch per chunk, and each of those is a full retain whose fixed cost dwarfs the work in a single chunk. The guessing fails on any body whose chunk boundaries sit on both blank lines and single newlines, which is ordinary markdown: on 30 such documents at the shipped defaults, 168 of 172 runs could not be rejoined — 2,518 sub-batches where 172 would do. Every native chunk is a verbatim substring of the body separated only by whitespace (except a JSON conversation array, whose chunks are re-serialized and which the merged-array candidate already covers), so _iter_located_chunks now pairs each streamed chunk with its span using a whitespace skip and a startswith, and the rejoin becomes body[start:end] — the original text rather than a guess at it. The candidate joins remain as fallbacks and the re-chunk verification is unchanged, so the #3282 alignment invariant still gates every slice. A run can also begin on the short tail that chunk packing leaves after recursing into an over-budget piece; that tail merges with its neighbour when re-chunked in isolation, so no join is faithful even byte-exact. _cut_run now retries the leading chunks the best candidate did reproduce, so one bad boundary costs one chunk instead of the whole run. Together: 2,518 -> 173 sub-batches on those documents, and 1,394 -> 90 on a single 2.7 MB body, at unchanged split cost. Claude-Session: https://claude.ai/code/session_01TbssiuHs32rLNMPYutRwub
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3790.
_iter_raw_sub_batchespacks an oversized item's native chunks into runs worthretain_batch_tokens, then has to rebuild the text of each run. It rebuilt it byguessing the separator — a merged JSON array,
"\n\n".join,"\n".join— andaccepted a guess only when re-chunking it reproduced the run exactly. When none did,
the run degraded to one sub-batch per chunk. Each of those is a full retain carrying
a fixed cost that dwarfs the work in a single chunk, so an ordinary document ended up
issuing ~30x the retains it needed.
What actually triggers it
Two independent things, and the first is not an edge case.
Separator rewriting.
"\n\n".joininserts blank lines where the body had singlenewlines. Any body whose chunk boundaries sit on both — an ordinary markdown
document — defeats every candidate. On 30 synthetic markdown-shaped documents at the
shipped defaults (
retain_chunk_size=3000,retain_batch_tokens=10_000), 168 of172 runs failed to rejoin: 2,518 sub-batches where 172 would do. The issue's
"5 of 18 runs" is the mild case.
Greedy-buffer reset.
_iter_recursive_splitsflushes its packing buffer beforerecursing into an over-budget piece, so the short tail it leaves never merges with the
next paragraph in the document — but it does when the run is re-chunked on its own.
A run beginning on such a tail has no faithful join at all, even a byte-exact one.
The fix
Carry the separators, by locating the chunks rather than threading them through the
chunker. Every native chunk is a verbatim substring of the body, separated only by
whitespace, for every content shape except a JSON conversation array (whose chunks are
re-serialized, and which the existing merged-array candidate already covers). So
_iter_located_chunkspairs each streamed chunk with its span using a whitespace skipand a
startswith— no search of the body — and the rejoin becomesbody[start:end]:the original text, separators and all, instead of a guess at them. The candidate joins
stay as fallbacks, and the re-chunk verification is unchanged, so the #3282 alignment
invariant still gates every slice.
Regroup after a boundary that cannot be rejoined. When no candidate rebuilds the
whole run,
_cut_runtakes the leading chunks the best candidate did reproduce andretries them as a run of their own. One bad boundary now costs one chunk instead of
the entire run.
Memory behaviour is unchanged: spans are two ints per chunk of a run already bounded
by
retain_batch_tokens, chunks are still streamed, and the candidates are now yieldedone at a time instead of all being materialised at once (#3756).
Measured
Sub-batches produced, at the shipped defaults:
Splitting costs the same — the common case now verifies on the first candidate
instead of the third, which offsets the extra work on the rare failing run. At the
~1.1 s fixed cost per retain measured in the issue, the 2.7 MB body goes from ~25 min
of retain overhead to ~1.6 min.
Structured bodies (JSON conversation arrays, JSONL, JSONL with blank lines) are
unaffected — they already rejoined, and produce the same split as before.
Tests
test_split_packs_a_run_whose_chunks_sit_on_mixed_separators— the markdown shapefrom the issue. 18 chunks → 18 sub-batches before, 2 after.
test_run_slices_regroup_after_a_boundary_that_cannot_be_rejoined— a run startingon a recursion tail ships
[1, rest], not one sub-batch per chunk.test_split_slices_are_whole_native_chunksgains amixed-separatorspayload, sothe alignment invariant is pinned for that shape alongside prose/JSON/JSONL.
The alignment invariant (every slice re-chunks to exactly its own chunks, and the
concatenation reproduces the document's native chunks) was additionally checked by
hand across prose, markdown, JSONL, JSONL-with-blank-lines, CRLF, unicode, tabs, a
200 KB single word, and JSON conversation arrays.
https://claude.ai/code/session_01TbssiuHs32rLNMPYutRwub