diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c2a5367d6..f6717d7669 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 0.23.4 + +### Enhancements + +- **Add `max_page` parameter to `chunk_by_title` for page-count-bounded chunking**: `chunk_by_title()` now accepts an optional `max_page: int` argument. When set, a chunk is closed and a new one started whenever an element's page number is more than `max_page - 1` pages past the page where the current chunk began. This gives callers a hard upper bound on how many pages a single chunk may span, independent of character or token limits. A `Title` element always resets the page counter (it already starts a new chunk via the existing boundary logic), so each section's page span is counted independently. The constraint is additive — it combines with all other `by_title` boundaries. Must be `>= 1`; raises `ValueError` otherwise. `max_page=1` is equivalent to the deprecated `multipage_sections=False`. +- **Deprecate `multipage_sections` in `chunk_by_title`**: `multipage_sections` is superseded by `max_page` and will be removed in a future release. It continues to work and emits a `DeprecationWarning`. Use `max_page=1` instead of `multipage_sections=False`; omit `max_page` entirely instead of `multipage_sections=True`. When both are provided, `max_page` takes priority. + ## 0.23.3 ### Fixes diff --git a/test_unstructured/chunking/test_title.py b/test_unstructured/chunking/test_title.py index 162d15de25..408be0c237 100644 --- a/test_unstructured/chunking/test_title.py +++ b/test_unstructured/chunking/test_title.py @@ -275,7 +275,8 @@ def test_chunk_by_title_separates_by_page_number(): Text("It is storming outside."), CheckBox(), ] - chunks = chunk_by_title(elements, multipage_sections=False, combine_text_under_n_chars=0) + with pytest.warns(DeprecationWarning, match="multipage_sections"): + chunks = chunk_by_title(elements, multipage_sections=False, combine_text_under_n_chars=0) assert len(chunks) == 5 assert chunks[0] == CompositeElement("A Great Day") @@ -304,7 +305,8 @@ def test_chuck_by_title_respects_multipage(): Text("It is storming outside."), CheckBox(), ] - chunks = chunk_by_title(elements, multipage_sections=True, combine_text_under_n_chars=0) + with pytest.warns(DeprecationWarning, match="multipage_sections"): + chunks = chunk_by_title(elements, multipage_sections=True, combine_text_under_n_chars=0) assert len(chunks) == 4 assert chunks[0] == CompositeElement( "A Great Day\n\nToday is a great day.\n\nIt is sunny outside." @@ -333,7 +335,8 @@ def test_chunk_by_title_groups_across_pages(): Text("It is storming outside."), CheckBox(), ] - chunks = chunk_by_title(elements, multipage_sections=True, combine_text_under_n_chars=0) + with pytest.warns(DeprecationWarning, match="multipage_sections"): + chunks = chunk_by_title(elements, multipage_sections=True, combine_text_under_n_chars=0) assert len(chunks) == 4 assert chunks[0] == CompositeElement( @@ -349,6 +352,124 @@ def test_chunk_by_title_groups_across_pages(): ) +def test_max_page_1_is_equivalent_to_multipage_sections_false(): + """max_page=1 produces identical output to the deprecated multipage_sections=False.""" + elements: list[Element] = [ + Title("A Great Day", metadata=ElementMetadata(page_number=1)), + Text("Today is a great day.", metadata=ElementMetadata(page_number=2)), + Text("It is sunny outside.", metadata=ElementMetadata(page_number=2)), + Title("An Okay Day"), + Text("Today is an okay day."), + ] + + chunks_new = chunk_by_title(elements, max_page=1, combine_text_under_n_chars=0) + with pytest.warns(DeprecationWarning): + chunks_old = chunk_by_title( + elements, multipage_sections=False, combine_text_under_n_chars=0 + ) + + assert chunks_new == chunks_old + + +def test_chunk_by_title_respects_max_page(): + """A chunk is closed when its elements would span more than max_page pages.""" + elements: list[Element] = [ + Title("Section A", metadata=ElementMetadata(page_number=1)), + Text("Page one text.", metadata=ElementMetadata(page_number=1)), + Text("Page two text.", metadata=ElementMetadata(page_number=2)), + Text("Page three text.", metadata=ElementMetadata(page_number=3)), + Text("Page four text.", metadata=ElementMetadata(page_number=4)), + Text("Page five text.", metadata=ElementMetadata(page_number=5)), + ] + + chunks = chunk_by_title(elements, max_page=2, combine_text_under_n_chars=0) + + # max_page=2: chunk 1 covers pages 1-2, page 3 triggers new chunk covering pages 3-4, + # page 5 triggers another new chunk. + assert len(chunks) == 3 + assert chunks[0] == CompositeElement( + "Section A\n\nPage one text.\n\nPage two text." + ) + assert chunks[1] == CompositeElement("Page three text.\n\nPage four text.") + assert chunks[2] == CompositeElement("Page five text.") + + +def test_chunk_by_title_max_page_resets_on_title(): + """Title boundaries reset the max_page page-counter so each section is counted independently.""" + elements: list[Element] = [ + Title("Section A", metadata=ElementMetadata(page_number=1)), + Text("Page one.", metadata=ElementMetadata(page_number=1)), + Title("Section B", metadata=ElementMetadata(page_number=2)), + Text("Page two.", metadata=ElementMetadata(page_number=2)), + Text("Page three.", metadata=ElementMetadata(page_number=3)), + Text("Page four.", metadata=ElementMetadata(page_number=4)), + ] + + chunks = chunk_by_title(elements, max_page=2, combine_text_under_n_chars=0) + + # Section A: pages 1 only → 1 chunk. + # Section B: pages 2-3 (2 pages) → 1 chunk, then page 4 triggers a new chunk. + assert len(chunks) == 3 + assert chunks[0] == CompositeElement("Section A\n\nPage one.") + assert chunks[1] == CompositeElement("Section B\n\nPage two.\n\nPage three.") + assert chunks[2] == CompositeElement("Page four.") + + +def test_chunk_by_title_max_page_1_breaks_on_every_page(): + """max_page=1 produces one chunk per page-group, breaking on every page change.""" + elements: list[Element] = [ + Title("Intro", metadata=ElementMetadata(page_number=1)), + Text("Page one.", metadata=ElementMetadata(page_number=1)), + Text("Page two.", metadata=ElementMetadata(page_number=2)), + Text("Page three.", metadata=ElementMetadata(page_number=3)), + ] + + chunks = chunk_by_title(elements, max_page=1, combine_text_under_n_chars=0) + + assert len(chunks) == 3 + assert chunks[0] == CompositeElement("Intro\n\nPage one.") + assert chunks[1] == CompositeElement("Page two.") + assert chunks[2] == CompositeElement("Page three.") + + +def test_chunk_by_title_max_page_not_undone_by_combiner(): + """PreChunkCombiner must not re-merge pre-chunks that were split by max_page. + + By default combine_text_under_n_chars == max_characters (500). Without the page-span + guard in PreChunk.can_combine(), the combiner would merge small pre-chunks back + together, silently violating the max_page limit. + """ + elements: list[Element] = [ + Title("Section", metadata=ElementMetadata(page_number=1)), + Text("short", metadata=ElementMetadata(page_number=1)), + Text("short", metadata=ElementMetadata(page_number=2)), + Text("short", metadata=ElementMetadata(page_number=3)), + Text("short", metadata=ElementMetadata(page_number=4)), + ] + + # Do NOT pass combine_text_under_n_chars=0 — that's the whole point: the combiner + # must be blocked by the max_page guard, not suppressed manually by the caller. + chunks = chunk_by_title(elements, max_page=2) + + assert len(chunks) == 2 + assert chunks[0] == CompositeElement("Section\n\nshort\n\nshort") + assert chunks[1] == CompositeElement("short\n\nshort") + + +def test_chunk_by_title_max_page_none_does_not_add_page_boundary(): + """Omitting max_page leaves all existing by-title behavior unchanged.""" + elements: list[Element] = [ + Title("Section", metadata=ElementMetadata(page_number=1)), + Text("Page one.", metadata=ElementMetadata(page_number=1)), + Text("Page five.", metadata=ElementMetadata(page_number=5)), + ] + + chunks = chunk_by_title(elements, combine_text_under_n_chars=0) + + assert len(chunks) == 1 + assert chunks[0] == CompositeElement("Section\n\nPage one.\n\nPage five.") + + def test_add_chunking_strategy_on_partition_html(): filename = "example-docs/example-10k-1p.html" chunk_elements = partition_html(filename, chunking_strategy="by_title") @@ -691,15 +812,45 @@ def it_does_not_complain_when_specifying_new_after_n_chars_by_itself(self): assert opts.soft_max == 200 + def it_translates_multipage_sections_false_to_max_page_1(self): + """multipage_sections=False is internally translated to max_page=1.""" + opts = _ByTitleChunkingOptions(multipage_sections=False) + assert opts.max_page == 1 + + def it_leaves_max_page_none_when_multipage_sections_is_true(self): + opts = _ByTitleChunkingOptions(multipage_sections=True) + assert opts.max_page is None + + def it_prefers_max_page_over_multipage_sections_when_both_are_set(self): + """max_page takes priority; multipage_sections is ignored without error.""" + opts = _ByTitleChunkingOptions(multipage_sections=False, max_page=3) + assert opts.max_page == 3 + + def it_emits_deprecation_warning_when_multipage_sections_is_used(self): + with pytest.warns(DeprecationWarning, match="multipage_sections.*deprecated"): + chunk_by_title([], multipage_sections=False) + + def it_does_not_warn_when_multipage_sections_is_not_passed(self): + import warnings + + with warnings.catch_warnings(): + warnings.simplefilter("error", DeprecationWarning) + chunk_by_title([]) # should not raise + @pytest.mark.parametrize( - ("multipage_sections", "expected_value"), - [(True, True), (False, False), (None, CHUNK_MULTI_PAGE_DEFAULT)], + ("max_page", "expected_value"), + [(1, 1), (3, 3), (None, None)], ) - def it_knows_whether_to_break_chunks_on_page_boundaries( - self, multipage_sections: bool, expected_value: bool + def it_accepts_valid_max_page_values( + self, max_page: Optional[int], expected_value: Optional[int] ): - opts = _ByTitleChunkingOptions(multipage_sections=multipage_sections) - assert opts.multipage_sections is expected_value + opts = _ByTitleChunkingOptions(max_page=max_page) + assert opts.max_page == expected_value + + @pytest.mark.parametrize("max_page", [0, -1, -10]) + def it_rejects_max_page_less_than_one(self, max_page: int): + with pytest.raises(ValueError, match=f"'max_page' argument must be >= 1, got {max_page}"): + _ByTitleChunkingOptions.new(max_page=max_page) # ================================================================================================ diff --git a/test_unstructured/partition/test_api.py b/test_unstructured/partition/test_api.py index 1206c259f0..e73312a4da 100644 --- a/test_unstructured/partition/test_api.py +++ b/test_unstructured/partition/test_api.py @@ -639,7 +639,6 @@ def expected_call_(): include_page_breaks=False, languages=None, max_characters=None, - multipage_sections=True, new_after_n_chars=None, ocr_languages=None, output_format=shared.OutputFormat.APPLICATION_JSON, diff --git a/unstructured/__version__.py b/unstructured/__version__.py index 697f1b365b..2aa76d8adb 100644 --- a/unstructured/__version__.py +++ b/unstructured/__version__.py @@ -1 +1 @@ -__version__ = "0.23.3" # pragma: no cover +__version__ = "0.23.4" # pragma: no cover diff --git a/unstructured/chunking/base.py b/unstructured/chunking/base.py index 64d5362ecd..5855ddb791 100644 --- a/unstructured/chunking/base.py +++ b/unstructured/chunking/base.py @@ -172,6 +172,16 @@ def combine_text_under_n_chars(self) -> int: arg_value = self._kwargs.get("combine_text_under_n_chars") return arg_value if arg_value is not None else 0 + @cached_property + def max_page(self) -> Optional[int]: + """Maximum number of pages a chunk may span; `None` means no page-span limit. + + Overridden by `_ByTitleChunkingOptions` to respect the `max_page` argument. The default + here is `None` so that `PreChunk.can_combine()` can check this property without needing to + know which chunking strategy is in use. + """ + return None + @cached_property def hard_max(self) -> int: """The maximum size for a chunk (in characters or tokens depending on mode). @@ -433,7 +443,7 @@ class PreChunker: - **Segregate semantic units.** Identify semantic unit boundaries and segregate elements on either side of those boundaries into different sections. In this case, the primary indicator of a semantic boundary is a `Title` element. A page-break (change in page-number) is also a - semantic boundary when `multipage_sections` is `False`. + semantic boundary when `max_page` is set. - **Minimize chunk count for each semantic unit.** Group the elements within a semantic unit into sections as big as possible without exceeding the chunk window size. @@ -681,6 +691,17 @@ def can_combine(self, pre_chunk: PreChunk) -> bool: return False if len(self._text) >= self._opts.combine_text_under_n_chars: return False + # -- refuse to combine when doing so would violate the max_page page-span limit. + # -- This prevents PreChunkCombiner from undoing boundaries placed by the + # -- `is_on_page_exceeding_max` predicate. + if self._opts.max_page is not None: + self_span = self._page_span + other_span = pre_chunk._page_span + if self_span is not None and other_span is not None: + combined_first = min(self_span[0], other_span[0]) + combined_last = max(self_span[1], other_span[1]) + if combined_last - combined_first + 1 > self._opts.max_page: + return False # -- avoid duplicating length computations by doing a trial-combine which is just as # -- efficient and definitely more robust than hoping two different computations of combined # -- length continue to get the same answer as the code evolves. Only possible because @@ -731,6 +752,20 @@ def overlap_tail(self) -> str: overlap = self._opts.inter_chunk_overlap return self._text[-overlap:].strip() if overlap else "" + @cached_property + def _page_span(self) -> tuple[int, int] | None: + """Return `(first_page, last_page)` across elements in this pre-chunk. + + Returns `None` when no element carries a page number, in which case no page-span + enforcement is possible and the caller should not block combination on that basis. + """ + pages = [ + e.metadata.page_number + for e in self._elements + if e.metadata.page_number is not None + ] + return (min(pages), max(pages)) if pages else None + def _iter_text_segments(self) -> Iterator[str]: """Generate overlap text and each element text segment in order. @@ -1874,6 +1909,54 @@ def page_number_incremented(element: Element) -> bool: return page_number_incremented +def is_on_page_exceeding_max(max_page: int) -> BoundaryPredicate: + """Returns a predicate that fires when a chunk would span more than `max_page` pages. + + The lifetime of the returned callable cannot extend beyond a single element-stream because it + stores current state (chunk-start page and current page) that is particular to that stream. + + The returned predicate tracks the page where the current chunk began. It fires (returns True) + when the current element's page number is more than `max_page - 1` pages past the chunk-start + page, i.e. when adding the element would make the chunk span more than `max_page` pages. + + When it fires, it resets the chunk-start page to the current element's page so that the next + chunk begins fresh from that page. + + A `Title` element resets the chunk-start page without firing, because the `is_title` predicate + already handles the boundary there; this keeps the two predicates consistent. + """ + chunk_start_page: int = 1 + current_page: int = 1 + is_first: bool = True + + def page_count_exceeded(element: Element) -> bool: + nonlocal chunk_start_page, current_page, is_first + + page_number = element.metadata.page_number + + if is_first: + current_page = page_number or 1 + chunk_start_page = current_page + is_first = False + return False + + if page_number is not None: + current_page = page_number + + # A Title element resets the chunk-start page (is_title handles the actual boundary). + if isinstance(element, Title): + chunk_start_page = current_page + return False + + if current_page - chunk_start_page >= max_page: + chunk_start_page = current_page + return True + + return False + + return page_count_exceeded + + def is_title(element: Element) -> bool: """True when `element` is a `Title` element, False otherwise.""" return isinstance(element, Title) diff --git a/unstructured/chunking/dispatch.py b/unstructured/chunking/dispatch.py index bf96da4b51..b7c0c8609c 100644 --- a/unstructured/chunking/dispatch.py +++ b/unstructured/chunking/dispatch.py @@ -58,8 +58,9 @@ def add_chunking_strategy(func: Callable[_P, list[Element]]) -> Callable[_P, lis + "\n\tStrategy used for chunking text into larger or smaller elements." + "\n\tDefaults to `None` with optional arg of 'basic' or 'by_title'." + "\n\tAdditional Parameters:" - + "\n\t\tmultipage_sections" - + "\n\t\t\tIf True, sections can span multiple pages. Defaults to True." + + "\n\t\tmax_page" + + "\n\t\t\tMaximum pages a single chunk may span. Only applies to 'by_title' strategy." + + "\n\t\t\tUse max_page=1 to break on every page change. Defaults to None." + "\n\t\tcombine_text_under_n_chars" + "\n\t\t\tCombines elements (for example a series of titles) until a section" + "\n\t\t\treaches a length of n characters. Only applies to 'by_title' strategy." diff --git a/unstructured/chunking/title.py b/unstructured/chunking/title.py index 05e2e74dfa..77d7d6ea5c 100644 --- a/unstructured/chunking/title.py +++ b/unstructured/chunking/title.py @@ -5,16 +5,16 @@ from __future__ import annotations +import warnings from functools import cached_property from typing import Iterable, Iterator, Optional from unstructured.chunking.base import ( - CHUNK_MULTI_PAGE_DEFAULT, BoundaryPredicate, ChunkingOptions, PreChunkCombiner, PreChunker, - is_on_next_page, + is_on_page_exceeding_max, is_title, ) from unstructured.documents.elements import Element @@ -26,6 +26,7 @@ def chunk_by_title( combine_text_under_n_chars: Optional[int] = None, include_orig_elements: Optional[bool] = None, max_characters: Optional[int] = None, + max_page: Optional[int] = None, max_tokens: Optional[int] = None, multipage_sections: Optional[bool] = None, new_after_n_chars: Optional[int] = None, @@ -61,11 +62,21 @@ def chunk_by_title( max_characters Chunks elements text and text_as_html (if present) into chunks of length n characters (hard max). Mutually exclusive with `max_tokens`. + max_page + Maximum number of pages a single chunk may span. When set, a new chunk is started whenever + an element's page number is more than `max_page - 1` pages past the page where the current + chunk began. Elements without a page number are assumed to continue the current page. + `max_page=1` breaks a chunk on every page change; `max_page=None` (default) places no + page-count limit. Must be >= 1 when specified. Takes priority over `multipage_sections` + when both are provided. max_tokens Chunks elements into chunks of n tokens (hard max). Requires `tokenizer` to be specified. Mutually exclusive with `max_characters`. multipage_sections - If True, sections can span multiple pages. Defaults to True. + Deprecated. Use `max_page` instead. `multipage_sections=False` is equivalent to + `max_page=1`; `multipage_sections=True` (the default) is equivalent to leaving `max_page` + unset. When both `multipage_sections` and `max_page` are provided, `max_page` takes + priority. new_after_n_chars Cuts off new sections once they reach a length of n characters (soft max). Defaults to `max_characters` when not specified, which effectively disables any soft window. @@ -98,10 +109,20 @@ def chunk_by_title( `False` to allow tables to share pre-chunks with adjacent elements (the pre-#4307 behavior). """ + if multipage_sections is not None: + warnings.warn( + "'multipage_sections' is deprecated and will be removed in a future version. " + "Use 'max_page=1' instead of 'multipage_sections=False', " + "or omit 'max_page' entirely instead of 'multipage_sections=True'.", + DeprecationWarning, + stacklevel=2, + ) + opts = _ByTitleChunkingOptions.new( combine_text_under_n_chars=combine_text_under_n_chars, include_orig_elements=include_orig_elements, max_characters=max_characters, + max_page=max_page, max_tokens=max_tokens, multipage_sections=multipage_sections, new_after_n_chars=new_after_n_chars, @@ -136,24 +157,24 @@ class _ByTitleChunkingOptions(ChunkingOptions): A remedy to over-chunking caused by elements mis-identified as Title elements. Every Title element would start a new chunk and this setting mitigates that, at the expense of sometimes violating legitimate semantic boundaries. - multipage_sections - Indicates that page-boundaries should not be respected while chunking, i.e. elements - appearing on two different pages can appear in the same chunk. + max_page + Hard upper bound on pages per chunk. `max_page=1` breaks on every page change; + `max_page=None` (default) places no page-count limit. Takes priority over the + deprecated `multipage_sections` when both are provided. """ @cached_property def boundary_predicates(self) -> tuple[BoundaryPredicate, ...]: """The semantic-boundary detectors to be applied to break pre-chunks. - For the `by_title` strategy these are sections indicated by a title (section-heading), an - explicit section metadata item (only present for certain document types), and optionally - page boundaries. + For the `by_title` strategy these are sections indicated by a title (section-heading) and, + when `max_page` is set, page-count boundaries. `max_page=1` breaks on every page change. """ def iter_boundary_predicates() -> Iterator[BoundaryPredicate]: yield is_title - if not self.multipage_sections: - yield is_on_next_page() + if self.max_page is not None: + yield is_on_page_exceeding_max(self.max_page) return tuple(iter_boundary_predicates()) @@ -165,35 +186,44 @@ def combine_text_under_n_chars(self) -> int: - Defaults to `max_characters` when not specified. - Is reduced to `new_after_n_chars` when it exceeds that value. """ - # -- `combine_text_under_n_chars` defaults to `max_characters` when not specified -- arg_value = self._kwargs.get("combine_text_under_n_chars") return self.hard_max if arg_value is None else arg_value + @cached_property + def max_page(self) -> Optional[int]: + """Maximum number of pages a single chunk may span, or None for no page-count limit. + + `max_page` takes priority over `multipage_sections` when both are provided. + `multipage_sections=False` translates to `max_page=1` when `max_page` is not set. + """ + max_page_arg = self._kwargs.get("max_page") + if max_page_arg is not None: + return max_page_arg + # -- backward compat: multipage_sections=False → max_page=1 -- + if self._kwargs.get("multipage_sections") is False: + return 1 + return None + @cached_property def multipage_sections(self) -> bool: - """When False, break pre-chunks on page-boundaries.""" - arg_value = self._kwargs.get("multipage_sections") - return CHUNK_MULTI_PAGE_DEFAULT if arg_value is None else bool(arg_value) + """Deprecated. Read-only backward-compat alias; returns False only when max_page == 1.""" + return self.max_page != 1 def _validate(self) -> None: """Raise ValueError if request option-set is invalid.""" - # -- start with base-class validations -- super()._validate() - # -- `combine_text_under_n_chars == 0` is valid (suppresses chunk combination) - # -- but a negative value is not if self.combine_text_under_n_chars < 0: raise ValueError( f"'combine_text_under_n_chars' argument must be >= 0," f" got {self.combine_text_under_n_chars}" ) - # -- `combine_text_under_n_chars` > `max_characters` can produce behavior confusing to - # -- users. The chunking behavior would be no different than when - # -- `combine_text_under_n_chars == max_characters`, but if `max_characters` is left to - # -- default (500) then it can look like chunk-combining isn't working. if self.combine_text_under_n_chars > self.hard_max: raise ValueError( f"'combine_text_under_n_chars' argument must not exceed `max_characters`" f" value, got {self.combine_text_under_n_chars} > {self.hard_max}" ) + + if self.max_page is not None and self.max_page < 1: + raise ValueError(f"'max_page' argument must be >= 1, got {self.max_page}")