Skip to content

Exp/ndjson elements file - #346

Draft
badGarnet wants to merge 3 commits into
mainfrom
exp/ndjson-elements-file
Draft

Exp/ndjson elements file#346
badGarnet wants to merge 3 commits into
mainfrom
exp/ndjson-elements-file

Conversation

@badGarnet

@badGarnet badGarnet commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Review in cubic

badGarnet and others added 3 commits July 28, 2026 19:37
…n memory

EXPERIMENT - for local editable-install testing, not for release.

The split-PDF path already streams each chunk response to a temp file without
reading it into memory (call_api_async), but recombination then made four full
copies of the document:

  1. json.load per chunk           (load_elements_from_response)
  2. all chunk lists held at once  (_elements_from_task_responses)
  3. flattened list                (same)
  4. json.dumps(elements).encode() (request_utils.create_response)

and general.partition then re-parsed that blob into PartitionResponse.elements
for a fifth. On a document with large image_base64 payloads this makes the
caller's peak worse than the server's.

This adds an opt-in path that never builds a list:

- PartitionAcceptEnum.APPLICATION_X_NDJSON. Previously the enum held only
  json/csv AND the response matcher rejected anything else, so NDJSON was not
  expressible at all.
- PartitionResponse.elements_file: path to an NDJSON file, set instead of
  `elements`. The CALLER owns deleting it.
- request_utils.combine_chunk_files_to_ndjson: concatenates the per-chunk temp
  files on disk. Detects each chunk's shape from its first non-space byte, so a
  json-array chunk is converted (bounded by one chunk, 20 pages by default) and
  an NDJSON chunk is copied through with no parsing at all.
- split_pdf_hook: records ndjson mode from the Accept header in before_request,
  collects chunk paths instead of parsing, and returns the combined path via an
  x-unstructured-elements-file header, following the existing convention where a
  cached chunk response carries its temp-file path as the body.

Elements-file mode requires cache_tmp_data (it needs the chunk files) and falls
back to the existing in-memory recombination otherwise. Default behavior is
unchanged: without the NDJSON accept header nothing here activates.

Note the combined file is deliberately written to the cache PARENT dir, not the
operation's TemporaryDirectory, which _clear_operation deletes immediately after
after_success returns.

Tests: 10 new unit tests for the combine helper (array/ndjson/mixed/empty
chunks, order, exact payload round-trip, non-ascii). Existing suite unaffected:
224 passed, 1 xfailed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ndjson_mode was gated on BOTH the Accept header AND cache_tmp_data. Those are
set by different parties -- the caller picks the Accept header, cache_tmp_data is
a separate split-PDF setting -- so they can disagree. When they did (NDJSON
requested, caching off), the server returned NDJSON while the hook took the JSON
path and res.json() raised on a body this client had itself asked for.

Observed on an SnD whose effective split policy reports cache_mode=disabled: the
partition node failed outright rather than degrading.

ndjson_mode now depends only on the Accept header, and both caching modes are
handled: cached chunks contribute their existing temp-file path, uncached chunks
spill their body verbatim via write_chunk_body_to_temp (no parsing) so the
combine step gets uniform input. Peak stays ~one chunk either way.

Tests: 2 new regression tests for the uncached path; suite 226 passed, 1 xfailed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Spilling each chunk body to disk did not reduce memory, because every response
object is retained in api_successful_responses for failure bookkeeping -- so the
in-memory body stayed alive alongside the file. On a 2500-page document split
into 125 chunks of 32 MB, the whole 4 GB accumulated anyway and the partitioner
plugin peaked at ~15 GiB against a 16 GiB limit.

Overwrite _content with the spill path after writing, mirroring what the cached
branch already does at the point of caching, so res.text means the same thing in
both branches and the payload is reclaimed immediately.

Measured on an SnD with cache_mode=disabled, 2500 pages / 125 chunks / 4 GB:
  buffered  -> 15252 MiB, job FAILED
  streaming -> 15814 MiB, job succeeded (before this fix)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant