Skip to content

perf(retain): rebuild a chunk run from the body instead of guessing its separators (#3790) - #3808

Draft
nicoloboschi wants to merge 1 commit into
mainfrom
fix/3790-run-rejoin-fragmentation
Draft

perf(retain): rebuild a chunk run from the body instead of guessing its separators (#3790)#3808
nicoloboschi wants to merge 1 commit into
mainfrom
fix/3790-run-rejoin-fragmentation

Conversation

@nicoloboschi

Copy link
Copy Markdown
Collaborator

Fixes #3790.

_iter_raw_sub_batches packs an oversized item's native chunks into runs worth
retain_batch_tokens, then has to rebuild the text of each run. It rebuilt it by
guessing the separator — a merged JSON array, "\n\n".join, "\n".join — and
accepted 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".join inserts blank lines where the body had single
newlines. 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 of
172 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_splits flushes its packing buffer before
recursing 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_chunks pairs each streamed chunk with its span using a whitespace skip
and a startswith — no search of the body — and the rejoin becomes body[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_run takes the leading chunks the best candidate did reproduce and
retries 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 yielded
one at a time instead of all being materialised at once (#3756).

Measured

Sub-batches produced, at the shipped defaults:

30 markdown docs one 2.7 MB body split time
before 2,518 1,394 205 ms
after 173 (172 runs is the floor) 90 205 ms

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 shape
    from the issue. 18 chunks → 18 sub-batches before, 2 after.
  • test_run_slices_regroup_after_a_boundary_that_cannot_be_rejoined — a run starting
    on a recursion tail ships [1, rest], not one sub-batch per chunk.
  • test_split_slices_are_whole_native_chunks gains a mixed-separators payload, so
    the 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

…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
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.

A run whose chunks cannot be rejoined fragments retain into one sub-batch per chunk

1 participant