Repository navigation
fix: send the content_type partition_via_api accepts - #4489
linhongyu510 wants to merge 8 commits into
Conversation
`content_type` has been the second positional parameter of `partition_via_api()` since the helper was introduced in Unstructured-IO#518, and the docstring describes it as "a string defining the file content in MIME type". It is never read: both `shared.Files(...)` call sites pass only `content` and `file_name`, so the value is silently discarded and the server falls back to sniffing the file name. `shared.Files` does have a `content_type` field, so nothing was missing on the SDK side -- only the hand-off. Populate it when the caller supplies a value: partition_via_api(filename=..., content_type="text/markdown") # before: files.content_type is None # after: files.content_type == "text/markdown" The field is left unset when no `content_type` is given, so the SDK keeps inferring the type from the file name exactly as before. The parameter is not moved, so existing positional calls are unaffected.
cragwolfe
left a comment
There was a problem hiding this comment.
Oracle Pro review (server-confirmed gpt-6-pro; completed verdict: NOT SAFE TO MERGE because of the SDK-floor regression) of head 383406198027de31c56387065a9372312c24789a against 0b7f806e0f6ef540df22fad5627756a72663eb11: one verified compatibility defect remains (inline). The MIME value reaches the file-part Content-Type with the locked SDK 0.43.2 and current SDK 0.46.2; omitted values preserve the existing serializer path.
The added tests assert only the object passed to mocked General.partition, so they do not establish multipart wire behavior or minimum-supported-SDK compatibility. Add coverage at the request boundary for explicit/omitted MIME types with filenames, bytes and streams, including the supported lower SDK bound.
Integration and release notes: GitHub currently reports mergeability as MERGEABLE, but its GitHub Actions CI, Security, CodeQL and benchmark runs are action_required; CI has no jobs. The external checks have success conclusions, but Cubic skipped this merge commit and Socket PR alerts skipped because dependencies did not change; they are not test execution evidence. The changelog entry is under historical 0.27.8 rather than the current 0.27.27-dev3 section; move it to the active release section.
Review only; no source changes or local tests.
(authored by codex)
|
|
||
| # -- `shared.Files` carries a `content_type` field; leave it unset when the caller did not | ||
| # -- specify one so the SDK keeps inferring the MIME type from the file name as before. | ||
| content_type_kwargs = {"content_type": content_type} if content_type is not None else {} |
There was a problem hiding this comment.
[P2] Preserve compatibility with the declared minimum SDK. pyproject.toml still permits unstructured-client==0.25.9, whose generated shared.Files is a dataclass with only content and file_name. For any non-None content_type, both construction paths now pass an unsupported keyword and raise TypeError before the HTTP request; the same call previously reached the API. SDK 0.26.0 introduces this field. Either raise the supported SDK minimum consistently, or use a compatible forwarding path (the API content_type form parameter is present in 0.25.9). Add minimum-version coverage and assert the serialized request so explicit types work throughout the advertised dependency range.
(authored by codex)
…25.9 compat shared.Files only gains content_type in unstructured-client 0.26.0; on the declared floor 0.25.9 the kwarg raises TypeError before the request. Route content_type through PartitionParameters (a multipart form field present on 0.25.9 and retained on newer SDKs) instead.
…CI job Address review follow-ups for the content_type forwarding fix: - R4: assert multipart serialization at the request boundary. The focused tests serialize the captured PartitionParameters and check the `content_type` form field (explicit) and its absence (omitted) across filename, bytes and stream(file=IO) inputs, plus caller keyword passthrough (strategy). A version-tolerant helper supports the 0.25.9 tuple serializer and the locked 0.43.2+ object serializer. - R3: add an isolated `test_unit_sdk_floor` CI job. It syncs the locked test environment, pins unstructured-client==0.25.9 (the advertised minimum), and runs only the focused wire tests; the ordinary locked suite is not downgraded. - R5: move the changelog entry from the historical 0.27.8 section to the active 0.27.27-dev3 section and describe the PartitionParameters form field approach instead of populating shared.Files.content_type.
|
Replying to the P2 SDK-floor note on Addressed with the compatible forwarding path rather than raising the SDK floor: 0.25.9 RED → GREEN (reproduced on installed
Matrix + negative control: the wire path (explicit MIME, omitted MIME, filename/bytes/stream, caller keyword passthrough) was verified on the locked SDK 0.43.2 and current 0.46.2 as well. The omitted-MIME negative control emits no Coverage: a new isolated Local checks: focused wire group 8 passed; full |
…pe-via-partition-parameters # Conflicts: # CHANGELOG.md
Problem
content_typehas been the second positional parameter ofpartition_via_api()since the helper was introduced in #518, and the docstring describes it as:It is never read. Both construction sites pass only
contentandfile_name:so the value is silently discarded and the server falls back to sniffing the file name — exactly the situation the parameter exists to override (an extensionless upload, a file whose extension lies about its type, or a stream where only the caller knows the format).
Measured on
main(3376cc96) by capturing the request handed to the SDK:This is not an SDK limitation:
shared.Fileshas acontent_typefield (['content', 'file_name', 'content_type']). Only the hand-off was missing.Fix
Populate the field when the caller supplies a value.
When no
content_typeis given the field is left unset rather than passed asNone, so the SDK keeps inferring the type from the file name exactly as before. The parameter is not moved, so existing positional calls bind unchanged.Tests
test_partition_via_api_sends_an_explicit_content_type_for_a_filenametest_partition_via_api_sends_an_explicit_content_type_for_a_filetest_partition_via_api_leaves_content_type_unset_when_not_given— guards the unchanged defaultThey assert on the request object actually handed to
General.partition, following themethod_mockpattern the surrounding tests already use.Verification
test_api.py(20 pre-existing + 3 new).test_api.py,test_md.py,test_text.pyandtest_json.pythe failure set is identical before and after; passing count goes 193 → 196, exactly the 3 added tests. (2 pre-existing failures are environmental and unrelated.)ruff checkandruff format --checkclean on both changed files.partition/api.py: none touches it (feat: add max_page to chunk_by_title and remove multipage_sections #4382 modifiestest_api.pyonly, for chunking).Independent of #4483 and #4488; all three are cut from
mainand touch different files.