Repository navigation
Conversation
Review findingsI found three Medium-priority compatibility regressions introduced by this refactor. The core unchunked rehydration behavior—preserving serialized element IDs and stored metadata—looks correct. [Medium] Preserve
|
2b5617d to
9e42507
Compare
|
Addressed all three compatibility findings in
I also advanced the development version to |
Signed-off-by: Rohan Patnaik <rohan-patnaik@users.noreply.github.com>
|
Resolved the new release conflicts in 09d7669: preserved main’s changelog entries and advanced this change to 0.27.13. The JSON implementation is unchanged. All 64 JSON tests, Ruff lint/format, and version-sync validation pass; GitHub now reports the PR mergeable. |
There was a problem hiding this comment.
Reviewed head 09d7669e5d8c0b022d06796a89a8433070551206 against base 74e2fea06836169ee7749d61ce7d555ce4b83051, including the full production Oracle Pro review and independent source verification. The three earlier findings are fixed: arbitrary-JSON originals retain timestamps, attachment filenames survive an explicit filename override, and registered chunkers receive the complete bound arguments on all four dispatch paths.
[Medium] Apply explicit timestamp overrides before chunking rehydrated elements
Affected code: unstructured/partition/json.py:142-155.
The rehydration branch calls _apply_chunking() before applying metadata_last_modified. Built-in chunkers create separate chunk metadata and retain the input elements in metadata.orig_elements (unstructured/chunking/base.py:878-880,961-977; tables retain a separate copy at 1185-1188). Updating the returned chunk at line 155 therefore leaves its originals with missing or stale timestamps.
For example:
chunks = partition_json(
text='[{"type":"NarrativeText","element_id":"source-id",'
'"text":"Saved document content."}]',
metadata_last_modified="2020-07-05T09:24:28",
chunking_strategy="basic",
)The returned chunk receives the explicit timestamp, but chunks[0].metadata.orig_elements[0].metadata.last_modified remains None. If the serialized element has an older timestamp, the original retains that older value. Before this refactor, the partition function applied the explicit timestamp before its chunking decorator ran. This breaks provenance for consumers that inspect or serialize retained originals.
Suggested fix: apply a truthy explicit metadata_last_modified to rehydrated input elements before _apply_chunking(). Keep the filename attachment guard separate and preserve stored timestamps when no explicit override is supplied; do not substitute the JSON container's filesystem timestamp. The post-chunk assignment can remain for custom chunkers that create their own elements.
Add a regression covering serialized elements with missing and existing timestamps under basic and by_title. Assert the override on both chunks and retained originals, including after serialization, and preserve existing no-override behavior. The current originals test covers arbitrary JSON; the serialized timestamp tests do not request chunking.
(authored by codex)
|
Fixed the explicit-timestamp provenance regression in 3f89166: rehydrated originals receive the override before chunking, while no-override timestamps stay unchanged. The eight new basic/by_title and serialization cases reproduce four failures before the fix; all 72 JSON tests, repository Ruff and version-sync checks now pass. Also merged current main and preserved its releases, advancing this change to 0.27.17. |
Signed-off-by: Rohan Patnaik <rohan-patnaik@users.noreply.github.com>
|
Merged current main into |
Signed-off-by: Rohan Patnaik <rohan-patnaik@users.noreply.github.com>
|
Merged current main in |
There was a problem hiding this comment.
Reviewed exact head cdcdb1d9606633e628a88f07b7ae7dd273f94695 against GitHub base 67f4cf1b7268068f1a58c09d1fb2f7045df6fc85 with a fresh, complete production Oracle Pro review (gpt-6-pro, Pro effort) and independent source verification. The four earlier issues are fixed: arbitrary JSON originals retain timestamps; rehydrated attachments retain their filenames; all four dispatch paths pass the complete bound arguments; and explicit timestamp overrides reach newly retained originals when rehydrated elements are chunked.
Three actionable compatibility issues remain. All are non-blocking by production impact; no High/Critical finding remains.
1. Medium, non-blocking — Explicit URL and HTML overrides are ignored during rehydration
unstructured/partition/json.py:153-158 applies only the filename and timestamp overrides. Calls such as partition_json(text=serialized, url="https://example.com/source") now retain an old URL or leave it absent. An explicit text_as_html is also ignored. Before this change, add_metadata() forwarded both values to add_element_metadata() (unstructured/file_utils/filetype.py:1050-1062). The arbitrary-JSON branch still supports them. Built-in chunkers filter these unsupported arguments, so forwarding them through _apply_chunking() does not apply the override.
Apply explicitly supplied non-None URL/HTML values to eligible rehydrated output, including explicit empty strings, while preserving the attachment exemption. Omitted/None values must preserve stored metadata. Add regression coverage for unchunked rehydration and both built-in strategies, with stored/absent values and attachments; unchunked IDs and unrelated metadata must remain intact.
2. Low, non-blocking — Arbitrary-JSON custom output loses attachment metadata
unstructured/partition/json.py:179-186 applies generic metadata to every custom-strategy result without the attachment guard used at line 155. A registered chunker may return an element with filename="invoice.pdf", filetype="application/pdf", and attached_to_filename="message.eml". With metadata_filename="renamed.eml", this branch changes its filename to renamed.eml and MIME type to application/json, while retaining the attachment marker. Supplied URL/HTML values can also overwrite attachment metadata. The removed metadata and filetype decorators both exempted attachment elements (filetype.py:1061,1091). Exposure is limited to registered chunkers that emit attachment metadata.
Restore the attachment guard around this metadata call. Test a registered strategy returning ordinary and attachment elements: generic metadata must apply to ordinary output, while the attachment retains its filename, MIME type, URL, HTML, and parent association.
3. Low, non-blocking — Empty-content custom output skips metadata and ID processing
unstructured/partition/json.py:95-103,121-129 returns the dispatch result directly. A custom strategy that emits a placeholder for zero input elements therefore bypasses generic metadata, hierarchy, and default deterministic-ID processing. Previously, the outer decorators processed that result after dispatch. Parsed [] still reaches lines 177-189 and processes output from the same empty-list callback. The existing chunk_by_something_else() fixture (test_unstructured/chunking/test_dispatch.py:75-87) supplies a concrete callback shape: it always returns one CompositeElement, even for zero inputs.
Route empty-content strategy results through the applicable generic processing used for parsed []. Compare empty text, whitespace-only path/stream input, and [] using a strategy that emits one element. Metadata and default deterministic IDs must agree; preserve unique_element_ids=True, stock empty results, and exception propagation.
These findings were verified by tracing the exact-head implementation, removed decorators, dispatch filtering, and checked-in tests. No local tests or source changes were performed.
(authored by codex)
cragwolfe
left a comment
There was a problem hiding this comment.
Approving reviewed head cdcdb1d9606633e628a88f07b7ae7dd273f94695 under Crag's publication policy. The fresh Oracle Pro review and independent verification found no blocking High/Critical defect. The four earlier issues are fixed; the three remaining Medium/Low compatibility findings are recorded in the consolidated non-blocking review.
(authored by codex)
|
Fixed the three compatibility findings in The new compatibility cases failed before the fix. All 200 focused JSON/chunking tests pass (7 optional skips). Ruff, formatting and version checks pass. |
Summary
Preserve serialized element IDs and source metadata when
partition_json()rehydrates Unstructured output. Generic JSON metadata and deterministic ID processing now apply only to arbitrary JSON, while explicit metadata overrides remain supported.Fixes #3365.
Verification
One existing deep-recursion expectation is excluded because it fails identically on current main under Python 3.13.