Repository navigation
fix(retain): preserve full document body when splitter chunks oversized input - #1855
Merged
Merged
Conversation
…ed input When a single retain content item exceeded HINDSIGHT_API_RETAIN_BATCH_TOKENS (~40 KB), `retain_batch_async` chunked it across multiple sub-batches and each sub-batch passed only its own slice to `handle_document_tracking`, which unconditionally upserts `documents.original_text`. The last sub-batch overwrote the body with its slice, so the persisted document body became a fragment of the input. Thread a `document_body_override` parameter from `_split_contents_into_sub_batches` through `_retain_batch_async_internal`, `retain_batch`, `_streaming_retain_batch`, `_try_delta_retain` and `_delta_metadata_only`. When set, the orchestrator uses it as `combined_content` for the doc-row write so every sub-batch persists the same full body (and computes the same `content_hash`, so the FOR-UPDATE takeover check still passes). The override is a reference to the splitter's source string — no extra copies, no extra RAM. Fixes #1838.
nicoloboschi
force-pushed
the
fix/document-body-partial-replace-1838
branch
from
May 29, 2026 09:49
e09eda1 to
4adca22
Compare
Contributor
|
This should fix #1797 |
nicoloboschi
added a commit
that referenced
this pull request
Jun 1, 2026
…cument (#1888) (#1896) * fix(retain): offset chunk_index across sub-batches of an oversized document (#1888) When retain_batch_async splits a single oversized item into multiple sub-batches (the in-process memory bound from #1571), all sub-batches share one document_id but each re-chunked its slice starting at chunk_index 0. The derived chunk_id ({bank}_{doc}_{index}) therefore collided across sub-batches, and store_chunks_batch's ON CONFLICT upsert overwrote earlier chunks. Only one sub-batch's worth of chunks/memories survived, while #1855 still wrote the full body to documents.original_text — so original_text and the chunks disagreed (Σ chunk_text ≈ one RETAIN_BATCH_TOKENS slice). Thread a per-document chunk_index_offset from the retain_batch_async sub-batch loop through _retain_batch_async_internal, retain_batch and _streaming_retain_batch. Each sequential sub-batch sharing a document_id now continues the chunk_index sequence instead of restarting at 0, so chunk_ids stay unique and every slice's chunks/memories are preserved. The offset is advanced by counting chunks with the same bank-resolved, strategy-applied chunk size the orchestrator uses (new _resolve_retain_chunk_size helper). Add tests asserting Σ chunk_text covers the full body and chunk_index is a contiguous 0..N-1 sequence, for both fresh and replacement oversized retains. Fixes #1888. * fix(retain): account for append-prepended body in sub-batch chunk offset (#1888) The chunk_index offset fix did not cover update_mode="append". For an oversized append, retain_batch prepends the existing document body to the first sub-batch as an extra content item before chunking, so that sub-batch occupies chunks(existing_body) extra chunk_index slots. The offset loop only counted the sub-batch's own content, so later sub-batches restarted too early and overwrote the first sub-batch's tail — dropping a chunk of the existing body plus new content per collision. Pre-fetch each append document's existing body up front (the first sub-batch overwrites original_text on commit, so it can't be read back afterwards), chunk it with the same resolved chunk size, and fold that count into the first sub-batch's offset. Add a regression test that appends an oversized body to a multi-chunk existing document and asserts chunk coverage spans existing+new (covers ~38% without the fix). Fixes #1888.
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.
Summary
Fixes #1838.
When a single retain content item exceeds
HINDSIGHT_API_RETAIN_BATCH_TOKENS(~40 KB),retain_batch_asyncchunks it across multiple sub-batches. Each sub-batch passed only its own slice tohandle_document_tracking, which unconditionally upsertsdocuments.original_text. The last sub-batch overwrote the body with its slice, so the persisted body became a fragment of the input (e.g. a 62 KB transcript stored as ~30 KB on Railway 0.7.1).Fix
Thread a
document_body_overrideparameter from_split_contents_into_sub_batchesthrough_retain_batch_async_internal,retain_batch,_streaming_retain_batch,_try_delta_retainand_delta_metadata_only. When set, the orchestrator uses it ascombined_contentfor the doc-row write so every sub-batch persists the same full body (and computes the samecontent_hash, so the FOR-UPDATE takeover check still passes).RAM: the override is a reference to the splitter's source string — no extra copies. The orchestrator clears
combined_contentafterhandle_document_trackingfinishes, same as before.Test plan
tests/test_large_document_replacement.py::test_large_same_id_replacement_preserves_full_body— small initial doc, large replacement under samedocument_id, assertsoriginal_textmatches exactly.tests/test_large_document_replacement.py::test_repeated_large_same_id_replacement_is_idempotent— repeats the replacement 3× and asserts each retain converges to the exact submitted body (acceptance criterion 4 from the issue).HINDSIGHT_API_RETAIN_BATCH_TOKENS=100and disables auto-consolidation so call time is sub-second.