Skip to content

feat: add enrichment origins metadata field - #16

Open
anurag6569201 wants to merge 1 commit into
qa/agent-unstructured-io-unstructured/pr-16-4370/basefrom
qa/agent-unstructured-io-unstructured/pr-16-4370/head
Open

anurag6569201 wants to merge 1 commit into
qa/agent-unstructured-io-unstructured/pr-16-4370/basefrom
qa/agent-unstructured-io-unstructured/pr-16-4370/head

Conversation

@anurag6569201

Copy link
Copy Markdown

Summary

Adds a new enrichment_origins field to ElementMetadata that tracks which model wrote (or contributed to) each enriched attribute.

What's new

  • ElementMetadata.enrichment_origins — a serialized dict[str, list[dict[str, str]]] mapping a written attribute name (e.g. text, text_as_html, embeddings) to a list of {"type", "provider", "model"} records, in application order. Authoring enrichments overwrite the list; additive enrichments append, preserving the prior author.
  • ConsolidationStrategy.DICT_LIST_UNIQUE — a new chunking consolidation strategy for dict[str, list] fields. It unions keys across elements and, per key, concatenates then dedupes records while preserving first-seen order. Wired up as the strategy for enrichment_origins.

Testing

  • Unit tests for the new metadata field (test_elements.py).
  • Unit tests for the DICT_LIST_UNIQUE merge behavior during chunking (test_base.py).

Summary by cubic

Adds per-attribute enrichment provenance via ElementMetadata.enrichment_origins and a dict-list merge strategy, and isolates per-chunk metadata so provenance updates can’t leak across split chunks.

  • New Features

    • ElementMetadata.enrichment_origins: maps an attribute to a list of {"type","provider","model"} in application order. Authoring overwrites; additive appends.
    • ConsolidationStrategy.DICT_LIST_UNIQUE: unions keys and per-key concat+dedupes while preserving order; used for enrichment_origins in chunking.
  • Bug Fixes

    • First and continuation split chunks each get a fresh metadata object.
    • Deep-copies enrichment_origins per chunk and sets is_continuation on later chunks, preventing any in-place updates from leaking to siblings or lazy continuations.

Written for commit 7a03b8c. Summary will update on new commits.

Review in cubic

Source merge-base: dedf144192347a89bb70ddc8a896eeeaa7440be0
Source head: 7a03b8c9886b6cbdbfcfe647984482f5daaa6724

@shipwright-agent

Copy link
Copy Markdown

✅ Shipwright · Approve

Recommendation: approve PR #16 · Tier T1
Checks: 0 total · 0 needing attention

Next step: ready to merge.

Findings (4)

  • HIGH The dedupe identity 'tuple(sorted(record.items()))' is computed twice per record (once for 'seen_ids', once for 'record_id') and rebuilds 'seen_ids' from scratch on every record vi · unstructured/chunking/base.py:914
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The DICT_LIST_UNIQUE dedupe uses 'tuple(sorted(record.items()))' as the record identity. · unstructured/chunking/base.py:912
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH '_chunk_metadata()' deep-copies only 'enrichment_origins', but other mutable metadata fields (e.g. · unstructured/chunking/base.py:866
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH 'enrichment_origins' is serialized verbatim into 'ElementMetadata.to_dict()' with no validation of record shape or content. · unstructured/documents/elements.py:168
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Fireworks usage: 13,110 input · 537 output · 13,647 total tokens · $0.0032 · 11s · 0 fix iteration(s)

Open the Shipwright check for full evidence and the audit bundle. Use /shipwright rerun to verify again.

yield field_name, " ".join(val.strip() for val in values)
# -- merge dict-of-list values: union keys, per key concatenate then dedupe
# -- records, preserving first-seen order --
elif strategy is CS.DICT_LIST_UNIQUE:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · HIGH

The dedupe identity 'tuple(sorted(record.items()))' is computed twice per record (once for 'seen_ids', once for 'record_id') and rebuilds 'seen_ids' from scratch on every record vi

Impact: The dedupe identity 'tuple(sorted(record.items()))' is computed twice per record (once for 'seen_ids', once for 'record_id') and rebuilds 'seen_ids' from scratch on every record via a set comprehension over all previously seen records. For a chunk consolidating many elements with long origin lists, this is O(n^2) in the number of records and will degrade noticeably on large documents.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

yield field_name, list(ordered_unique_keys.keys())
elif strategy is CS.STRING_CONCATENATE:
yield field_name, " ".join(val.strip() for val in values)
# -- merge dict-of-list values: union keys, per key concatenate then dedupe

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · HIGH

The DICT_LIST_UNIQUE dedupe uses 'tuple(sorted(record.items()))' as the record identity.

Impact: The DICT_LIST_UNIQUE dedupe uses 'tuple(sorted(record.items()))' as the record identity. This breaks if any record value is itself a dict or list (unhashable after tuple conversion), and silently treats records with non-string values or nested structures as distinct even when semantically equal. The type hint restricts to 'dict[str, str]', but nothing enforces it at runtime; a producer passing a nested dict will rai…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

The same metadata as the first text-split chunk but includes `.is_continuation = True`.
Unused for non-oversized pre-chunks since those are not subject to text-splitting.

A new object is produced on each call (not cached) because each continuation chunk needs

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · HIGH

'_chunk_metadata()' deep-copies only 'enrichment_origins', but other mutable metadata fields (e.g.

Impact: '_chunk_metadata()' deep-copies only 'enrichment_origins', but other mutable metadata fields (e.g. 'emphasized_text_contents', 'emphasized_text_tags', 'languages') remain shared across split chunks via the shallow copy. If any downstream code mutates those lists in place on one chunk, the mutation leaks to sibling chunks — the exact bug class this change fixes for 'enrichment_origins' but leaves open for other list…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

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