diff --git a/CHANGELOG.md b/CHANGELOG.md index eb910939..b62d9527 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.46.1 + +### Fixes +* Fail loudly when a split-PDF chunk returns HTTP 200 with an empty JSON body in NDJSON elements-file mode. The chunk used to be logged and skipped, so the combined `elements_file` was silently short by those pages while the call still returned 200 — and `split_pdf_allow_failed=False` did not catch it, because an empty 200 counts as a successful chunk. Recombination now raises `EmptyChunkResponseError` (a `ValueError`), matching the buffered path, which raises `JSONDecodeError` on the same response. Emptiness is judged against the chunk's own `Content-Type`: JSON has no empty document (a chunk with no elements is `[]`), while `application/x-ndjson` encodes zero records as zero lines, so an empty NDJSON chunk is well formed and still contributes nothing. + ## 0.46.0 ### Features diff --git a/RELEASES.md b/RELEASES.md index d07c7d53..d30e16cc 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1251,3 +1251,13 @@ Based on: - [python v0.46.0] . ### Releases - [PyPI v0.46.0] https://pypi.org/project/unstructured-client/0.46.0 - . + +## 2026-08-04 00:00:00 +### Changes +Based on: +- OpenAPI Doc +- Speakeasy CLI 1.601.0 (2.680.0) https://github.com/speakeasy-api/speakeasy +### Generated +- [python v0.46.1] . +### Releases +- [PyPI v0.46.1] https://pypi.org/project/unstructured-client/0.46.1 - . diff --git a/_test_unstructured_client/unit/test_ndjson_elements_file.py b/_test_unstructured_client/unit/test_ndjson_elements_file.py index 5aac271d..adab470f 100644 --- a/_test_unstructured_client/unit/test_ndjson_elements_file.py +++ b/_test_unstructured_client/unit/test_ndjson_elements_file.py @@ -118,22 +118,98 @@ def test_mixed_chunk_formats(tmp_path): @pytest.mark.parametrize("body", ["", " ", "\n\n"]) -def test_empty_chunk_files_are_skipped(tmp_path, body): +@pytest.mark.parametrize("media_type", [None, "application/json", "application/json; charset=utf-8"]) +def test_empty_json_chunk_files_raise(tmp_path, body, media_type): + """An empty 200 body from a JSON chunk must fail, not be skipped. + + JSON has no empty document -- a chunk with no elements is `[]` -- so this is a + malformed response. Skipping it made the document silently short: this function returns + only a path and an element count, so nothing downstream could tell a truncated document + from a complete one. `split_pdf_allow_failed` does not cover it either -- the chunk + reported success. An unset `Content-Type` is treated as JSON, which is what the + deployed API returns. + """ chunk_a = tmp_path / "a.json" chunk_empty = tmp_path / "empty.json" + _write_json_array(chunk_a, _elements("a", 2)) + chunk_empty.write_text(body, encoding="utf-8") + + out = tmp_path / "combined.ndjson" + with pytest.raises(request_utils.EmptyChunkResponseError, match="empty.json"): + combine_chunk_files_to_ndjson( + [str(chunk_a), str(chunk_empty)], str(out), ["application/json", media_type] + ) + + +def test_empty_chunk_media_types_default_to_json_when_not_passed(tmp_path): + """Callers that cannot supply media types must still get the strict reading.""" + chunk_empty = tmp_path / "empty.json" + chunk_empty.write_text("", encoding="utf-8") + + with pytest.raises(request_utils.EmptyChunkResponseError): + combine_chunk_files_to_ndjson([str(chunk_empty)], str(tmp_path / "combined.ndjson")) + + +def test_empty_chunk_error_is_a_value_error(tmp_path): + """Parity with the buffered path, which raises `JSONDecodeError` (a `ValueError`).""" + chunk_empty = tmp_path / "empty.json" + chunk_empty.write_text("", encoding="utf-8") + + with pytest.raises(ValueError): + combine_chunk_files_to_ndjson( + [str(chunk_empty)], str(tmp_path / "combined.ndjson"), ["application/json"] + ) + + +@pytest.mark.parametrize("body", ["", " ", "\n\n"]) +@pytest.mark.parametrize( + "media_type", ["application/x-ndjson", "application/x-ndjson; charset=utf-8", "APPLICATION/X-NDJSON"] +) +def test_empty_ndjson_chunk_is_zero_records(tmp_path, body, media_type): + """NDJSON spells zero records as zero lines, so an empty body is well formed. + + The counterpart to `test_empty_json_chunk_files_raise`: the same empty file on disk is + a defect from a JSON chunk and a legitimate result from an NDJSON one, and the declared + media type is the only thing that separates them. Failing here would break a split + whose pages are blank. + """ + chunk_a = tmp_path / "a.ndjson" + chunk_empty = tmp_path / "empty.ndjson" elements_a = _elements("a", 2) - _write_json_array(chunk_a, elements_a) + _write_ndjson(chunk_a, elements_a) chunk_empty.write_text(body, encoding="utf-8") out = tmp_path / "combined.ndjson" - written = combine_chunk_files_to_ndjson([str(chunk_a), str(chunk_empty)], str(out)) + written = combine_chunk_files_to_ndjson( + [str(chunk_a), str(chunk_empty)], str(out), ["application/x-ndjson", media_type] + ) assert written == 2 assert _read_ndjson(out) == elements_a +def test_every_ndjson_chunk_empty_yields_an_empty_output(tmp_path): + """A document whose every chunk produced no elements is empty, not an error.""" + chunk_a = tmp_path / "a.ndjson" + chunk_b = tmp_path / "b.ndjson" + chunk_a.write_text("", encoding="utf-8") + chunk_b.write_text("", encoding="utf-8") + + out = tmp_path / "combined.ndjson" + written = combine_chunk_files_to_ndjson( + [str(chunk_a), str(chunk_b)], str(out), ["application/x-ndjson"] * 2 + ) + + assert written == 0 + assert out.read_text(encoding="utf-8") == "" + + def test_empty_array_chunk_contributes_nothing(tmp_path): - """A chunk that legitimately produced no elements (e.g. blank pages).""" + """A chunk that legitimately produced no elements (e.g. blank pages). + + The JSON counterpart to `test_empty_ndjson_chunk_is_zero_records`: `[]` and an empty + body are different responses on the wire, and only the latter is a defect. + """ chunk_a = tmp_path / "a.json" chunk_b = tmp_path / "b.json" _write_json_array(chunk_a, []) @@ -380,16 +456,31 @@ def test_server_cannot_name_a_local_file_via_a_response_header(tmp_path): # --- hook-level temp-file lifecycle ------------------------------------------------ -def _ndjson_response(elements): +def _ndjson_response(elements, media_type="application/x-ndjson"): body = "".join(json.dumps(e) + "\n" for e in elements).encode() - return httpx.Response(status_code=200, content=body) + headers = {"content-type": media_type} if media_type else {} + return httpx.Response(status_code=200, headers=headers, content=body) + +def _cached_chunk_response(path, media_type="application/x-ndjson"): + """What the cached path leaves behind: the body replaced by its temp-file path. -def _hook_in_ndjson_mode(operation_id, tmp_path): + Mirrors `_await_elements`' cached branch, which streams the body to disk and rebuilds + the response with `content=temp_file_name` and the server's original headers -- so the + declared media type still describes the file's contents, not the path in the body. + """ + return httpx.Response( + status_code=200, + headers={"content-type": media_type} if media_type else {}, + content=str(path).encode(), + ) + + +def _hook_in_ndjson_mode(operation_id, tmp_path, cache_tmp_data=False): """A hook set up as `before_request` would leave it for an uncached NDJSON run.""" hook = SplitPdfHook() hook.ndjson_mode[operation_id] = True - hook.cache_tmp_data_feature[operation_id] = False + hook.cache_tmp_data_feature[operation_id] = cache_tmp_data hook.cache_tmp_data_dir[operation_id] = str(tmp_path) hook.allow_failed[operation_id] = False # Marks the operation live; `_clear_operation` removing it is what signals teardown. @@ -546,6 +637,76 @@ def test_malformed_chunk_leaves_no_partial_output_behind(tmp_path): assert list(Path(tmp_path).glob("*.partial")) == [] +@pytest.mark.parametrize("media_type", ["application/json", None]) +def test_empty_json_chunk_response_fails_the_operation(tmp_path, media_type): + """An empty 200 JSON chunk must fail the whole partition, as the buffered path does. + + Driven through the hook because that is where the consequence lives: previously the + chunk was skipped and `_build_after_success_response` handed back a combined file that + was short by those pages, with a 200 alongside it. Also asserts no partial or spilled + file survives, since the raise happens mid-recombination. + """ + operation_id = "op-empty-chunk" + hook = _hook_in_ndjson_mode(operation_id, tmp_path) + headers = {"content-type": media_type} if media_type else {} + responses = [ + (0, _ndjson_response(_elements("a", 2))), + (1, httpx.Response(status_code=200, headers=headers, content=b"")), + ] + + with pytest.raises(request_utils.EmptyChunkResponseError): + hook._elements_from_task_responses(operation_id, responses, started_at=0.0) + + assert operation_id not in hook.ndjson_output_path + assert _ndjson_files_in(tmp_path) == [] + assert list(Path(tmp_path).glob("*.partial")) == [] + + +def test_empty_ndjson_chunk_response_is_accepted_in_memory_mode(tmp_path): + """A zero-record NDJSON chunk is a valid result and must not fail the partition. + + The default path (`cache_tmp_data` off): the empty body is spilled to disk, so the + media type carried by the response is the only thing that distinguishes it from the + malformed JSON case above. + """ + operation_id = "op-empty-ndjson-memory" + hook = _hook_in_ndjson_mode(operation_id, tmp_path) + elements_a, elements_c = _elements("a", 2), _elements("c", 3) + responses = [ + (0, _ndjson_response(elements_a)), + (1, _ndjson_response([])), # a chunk of blank pages + (2, _ndjson_response(elements_c)), + ] + + hook._elements_from_task_responses(operation_id, responses, started_at=0.0) + + combined = hook.ndjson_output_path[operation_id] + assert _read_ndjson(combined) == elements_a + elements_c + # The spilled chunk files are gone; only the combined output survives. + assert _ndjson_files_in(tmp_path) == [Path(combined).name] + + +def test_empty_ndjson_chunk_response_is_accepted_in_cached_mode(tmp_path): + """Cached counterpart: the chunk file on disk is empty and the body is its path.""" + operation_id = "op-empty-ndjson-cached" + hook = _hook_in_ndjson_mode(operation_id, tmp_path, cache_tmp_data=True) + # Cached chunk files live in the operation's tempdir, kept out of the directory the + # combined file lands in so the assertion below stays unambiguous. + cache_dir = tmp_path / "cache" + cache_dir.mkdir() + elements_a = _elements("a", 2) + chunk_a, chunk_empty = cache_dir / "a.json", cache_dir / "empty.json" + _write_ndjson(chunk_a, elements_a) + chunk_empty.write_text("", encoding="utf-8") + responses = [(0, _cached_chunk_response(chunk_a)), (1, _cached_chunk_response(chunk_empty))] + + hook._elements_from_task_responses(operation_id, responses, started_at=0.0) + + combined = hook.ndjson_output_path[operation_id] + assert _read_ndjson(combined) == elements_a + assert _ndjson_files_in(tmp_path) == [Path(combined).name] + + def test_no_combined_file_is_created_when_every_chunk_failed(tmp_path): operation_id = "op-all-failed" hook = _hook_in_ndjson_mode(operation_id, tmp_path) diff --git a/src/unstructured_client/_hooks/custom/request_utils.py b/src/unstructured_client/_hooks/custom/request_utils.py index 66bdaacd..f15c3580 100644 --- a/src/unstructured_client/_hooks/custom/request_utils.py +++ b/src/unstructured_client/_hooks/custom/request_utils.py @@ -290,6 +290,28 @@ def create_response(elements: list) -> httpx.Response: _SNIFF_BLOCK_SIZE = 64 +class EmptyChunkResponseError(ValueError): + """A split-PDF chunk returned HTTP 200 with a body its own media type cannot produce. + + Whether an empty body is a defect depends on the format the chunk claims. JSON has no + empty document -- a chunk with no elements is `[]` -- so nothing at all is malformed, + and skipping it would make the document silently short with no way for the caller to + notice (`split_pdf_allow_failed` does not cover it, because the chunk reports + success). NDJSON is a record per line, so an empty body is a well-formed zero-record + response and is accepted. + + Subclasses `ValueError` so that it lands where the buffered path's `JSONDecodeError` + (also a `ValueError`) already does for callers that guard the parse. + """ + + +def _is_ndjson_media_type(media_type: Optional[str]) -> bool: + """Whether a chunk's `Content-Type` declares NDJSON, ignoring any parameters.""" + if not media_type: + return False + return media_type.split(";", 1)[0].strip().lower() == NDJSON_MEDIA_TYPE + + def _first_non_space_char(stream: TextIO) -> str: """Return the first non-whitespace character in `stream`, or "" if there is none.""" while True: @@ -301,7 +323,11 @@ def _first_non_space_char(stream: TextIO) -> str: return stripped[0] -def combine_chunk_files_to_ndjson(chunk_paths: list[str], out_path: str) -> int: +def combine_chunk_files_to_ndjson( + chunk_paths: list[str], + out_path: str, + media_types: Optional[list[Optional[str]]] = None, +) -> int: """Combine per-chunk split-PDF response files into one NDJSON file on disk. Recombining chunks by parsing them builds four full copies of the document (a list @@ -316,27 +342,57 @@ def combine_chunk_files_to_ndjson(chunk_paths: list[str], out_path: str) -> int: NDJSON chunks are copied through without parsing. + An empty body is read against the chunk's declared media type, which is the only + thing that distinguishes the two cases: NDJSON encodes zero records as zero lines, so + an empty NDJSON chunk contributes nothing and is fine; JSON has no empty document, so + an empty one is malformed and raises rather than silently shortening the result. + A chunk whose media type is unknown is treated as JSON -- that is what the deployed + API returns, and guessing NDJSON would reinstate the silent truncation. + Args: chunk_paths: Per-chunk response files, in element order. out_path: File to write the combined NDJSON to. + media_types: The `Content-Type` each chunk was served with, positionally matching + `chunk_paths`. Omit when the media types are not known. Returns: The number of elements written. + + Raises: + EmptyChunkResponseError: A non-NDJSON chunk returned 200 with an empty body. """ total = 0 with open(out_path, "w", encoding="utf-8") as out: - for chunk_path in chunk_paths: + for index, chunk_path in enumerate(chunk_paths): + media_type = media_types[index] if media_types is not None else None with open(chunk_path, "r", encoding="utf-8") as chunk: first_char = _first_non_space_char(chunk) if not first_char: - # A 200 with an empty body. Log it, because otherwise the document is - # silently short and the element count cannot be reconciled against - # the chunk count. - logger.warning( - "split_pdf event=ndjson_empty_chunk file=%s", + if _is_ndjson_media_type(media_type): + # Zero records, spelled the only way NDJSON can spell it. + logger.debug( + "split_pdf event=ndjson_empty_chunk file=%s media_type=%s", + os.path.basename(chunk_path), + media_type, + ) + continue + # A JSON 200 with an empty body. Fail loudly: skipping it makes the + # document silently short with no way for the caller to notice, since + # this function's only output is the combined path. The buffered path + # raises `JSONDecodeError` on the same response, so NDJSON mode must + # not turn a hard failure into truncation. + logger.error( + "split_pdf event=empty_chunk_body file=%s media_type=%s", os.path.basename(chunk_path), + media_type, + ) + raise EmptyChunkResponseError( + "A split-PDF chunk returned HTTP 200 with an empty body " + f"({os.path.basename(chunk_path)}, Content-Type: " + f"{media_type or 'unset'}); its elements would be missing from the " + "combined output. A JSON chunk with no elements must be an empty " + "array." ) - continue chunk.seek(0) if first_char == "[": diff --git a/src/unstructured_client/_hooks/custom/split_pdf_hook.py b/src/unstructured_client/_hooks/custom/split_pdf_hook.py index fed214d3..c0f6920f 100644 --- a/src/unstructured_client/_hooks/custom/split_pdf_hook.py +++ b/src/unstructured_client/_hooks/custom/split_pdf_hook.py @@ -1419,6 +1419,10 @@ def _elements_from_task_responses( elements = [] ndjson_mode = self.ndjson_mode.get(operation_id, False) chunk_paths: list[str] = [] + # Positionally matches `chunk_paths`. Recombination needs each chunk's declared + # format to tell a well-formed zero-record NDJSON body from a malformed empty JSON + # one; both are an empty file on disk. + chunk_media_types: list[Optional[str]] = [] # Subset of `chunk_paths` this method created itself, and so must clean up. The # rest belong to the operation's tempdir and are removed with it. spilled_chunk_paths: list[str] = [] @@ -1431,6 +1435,9 @@ def _elements_from_task_responses( ) successful_responses.append(res) if ndjson_mode: + # Read before the cached branch overwrites the body with a path; the + # header still describes what the server actually sent. + chunk_media_types.append(res.headers.get("Content-Type")) # Neither branch parses the body; that is what keeps peak memory at # roughly one chunk during recombination. if self.cache_tmp_data_feature.get(operation_id, DEFAULT_CACHE_TMP_DATA): @@ -1496,7 +1503,7 @@ def _elements_from_task_responses( if ndjson_mode: try: if chunk_paths: - self._combine_chunks_to_ndjson(operation_id, chunk_paths) + self._combine_chunks_to_ndjson(operation_id, chunk_paths, chunk_media_types) finally: # These are ours; the cached chunk files belong to the operation tempdir. _unlink_quietly(spilled_chunk_paths) @@ -1505,7 +1512,12 @@ def _elements_from_task_responses( flattened_elements = [element for sublist in elements for element in sublist] return flattened_elements - def _combine_chunks_to_ndjson(self, operation_id: str, chunk_paths: list[str]) -> None: + def _combine_chunks_to_ndjson( + self, + operation_id: str, + chunk_paths: list[str], + chunk_media_types: Optional[list[Optional[str]]] = None, + ) -> None: """Concatenate the per-chunk files into one NDJSON file and record its path. `_build_after_success_response` turns the recorded path into the response. The @@ -1523,7 +1535,9 @@ def _combine_chunks_to_ndjson(self, operation_id: str, chunk_paths: list[str]) - fd, staging_path = tempfile.mkstemp(suffix=".ndjson.partial", dir=temp_dir_path) os.close(fd) try: - written = request_utils.combine_chunk_files_to_ndjson(chunk_paths, staging_path) + written = request_utils.combine_chunk_files_to_ndjson( + chunk_paths, staging_path, chunk_media_types + ) # Same directory, so this is atomic; the caller never sees a partial file. os.replace(staging_path, out_path) except BaseException: diff --git a/src/unstructured_client/_version.py b/src/unstructured_client/_version.py index 6fa28865..02e1257b 100644 --- a/src/unstructured_client/_version.py +++ b/src/unstructured_client/_version.py @@ -3,10 +3,10 @@ import importlib.metadata __title__: str = "unstructured-client" -__version__: str = "0.46.0" +__version__: str = "0.46.1" __openapi_doc_version__: str = "1.2.31" __gen_version__: str = "2.680.0" -__user_agent__: str = "speakeasy-sdk/python 0.46.0 2.680.0 1.2.31 unstructured-client" +__user_agent__: str = "speakeasy-sdk/python 0.46.1 2.680.0 1.2.31 unstructured-client" try: if __package__ is not None: