From 9bdb1cfba5032b49623c76617910c44d3101df5f Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Tue, 18 Aug 2026 20:14:34 -0400 Subject: [PATCH] Keep figures in the chunk they came from Refs #1277 Images extracted from Word and PowerPoint files were appended as extra chunks at the end of the document, with page numbers continuing past the real content. A figure on page 5 of a 15-page document became chunk 16, so a search hit on the figure lost its surrounding text and a citation pointed at a page that does not exist. Merging rather than appending also removes a latent indexing hazard. Chunk ids are derived from the page number, so emitting a second chunk that reuses page 5 would have overwritten the original in the search index. Image content is therefore folded into the existing chunk's content, which is both what is wanted and the only safe option. Placement is resolved per source. PowerPoint images follow the slide that references them, mapped onto the chunk covering that slide so grouped slides work. Word images are located by walking document.xml in reading order and counting the words that precede each image reference, then mapped proportionally onto the word-count chunks; proportional rather than absolute because the extractor's word count does not match the raw body exactly, and an absolute offset would drift and cluster every image at the front. Legacy .doc and .ppt images carry no recoverable position, so they anchor to the final chunk instead of inventing a page beyond the document. Merged chunks are held under a size budget derived from the chunk size cap; anything that does not fit spills to a trailing chunk rather than producing an oversized chunk. PDFs were already correct. Content Understanding attributes each figure to its page by span and Document Intelligence Layout inlines tables and figures into the page markdown, so equations and tables already stayed on their page. That behavior is unchanged and now has a regression test. Verifying the Word offsets against a real document first caught a bug in the groundwork: relationship targets are written as media/image1.emf in Word but ../media/image1.png in PowerPoint, and the shared normalizer produced word/media/media/image1.emf for the Word form, so nothing matched. Both now resolve through one target normalizer. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- application/single_app/config.py | 2 +- application/single_app/functions_documents.py | 215 +++++++- .../single_app/functions_office_media.py | 129 ++++- ...NTENT_UNDERSTANDING_ENHANCED_EXTRACTION.md | 35 +- docs/explanation/release_notes.md | 12 + .../test_figure_chunk_association.py | 465 ++++++++++++++++++ 6 files changed, 825 insertions(+), 33 deletions(-) create mode 100644 functional_tests/test_figure_chunk_association.py diff --git a/application/single_app/config.py b/application/single_app/config.py index 1e8d4e158..87f1177ed 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -96,7 +96,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.250.227" +VERSION = "0.250.228" IS_DEVELOPMENT = is_development_env_enabled() SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax') diff --git a/application/single_app/functions_documents.py b/application/single_app/functions_documents.py index a4a7fcf92..d10d4a769 100644 --- a/application/single_app/functions_documents.py +++ b/application/single_app/functions_documents.py @@ -258,6 +258,12 @@ def _resolve_metadata_extraction_client(settings, identity_context=None): # treats them as a signal that Enhanced extraction is worth the extra cost, because Content # Understanding is the only engine that describes figures. DI_MARKDOWN_FIGURE_PATTERN = re.compile(r'(|!\[[^\]]*\]\()', re.IGNORECASE) +# Budget for image content merged into an existing chunk. The cap is token-oriented, so it is +# converted with a conservative characters-per-token estimate before being used as a length limit. +OFFICE_IMAGE_MERGE_CHARS_PER_TOKEN = 4 +OFFICE_IMAGE_MERGE_UTILIZATION = 0.9 +OFFICE_IMAGE_MERGE_FALLBACK_CAP = 16384 +OFFICE_IMAGE_MERGE_MIN_CHAR_LIMIT = 4000 def is_pdf_file_name(file_name): @@ -339,17 +345,18 @@ def _build_office_embedded_image_chunks( file_path, settings, update_callback, - starting_page_number=1, ): - """Analyze images embedded in an Office file and return them as labelled content chunks. + """Analyze images embedded in an Office file and return placeable content blocks. - Neither engine describes figures inside Office files, so the images are pulled out of the OOXML - package and analyzed individually with whichever engine backs the admin's selected mode. + Neither engine describes figures inside Office files, so the images are pulled out of the + package and analyzed individually with whichever engine backs the admin's selected mode. Each + result carries the position metadata needed to merge it back into the chunk it came from, + rather than being appended to the end of the document. - Returns ``(chunks, analyzed_count, extraction_engine)``. + Returns ``(image_blocks, analyzed_count, extraction_engine, total_body_words)``. """ if not settings.get('enable_office_embedded_image_analysis', True): - return [], 0, EXTRACTION_ENGINE_DOCUMENT_INTELLIGENCE + return [], 0, EXTRACTION_ENGINE_DOCUMENT_INTELLIGENCE, 0 min_pixels = normalize_office_embedded_image_min_pixels( settings.get('office_embedded_image_min_pixels') @@ -358,7 +365,7 @@ def _build_office_embedded_image_chunks( settings.get('office_embedded_image_max_per_document') ) if max_images <= 0: - return [], 0, EXTRACTION_ENGINE_DOCUMENT_INTELLIGENCE + return [], 0, EXTRACTION_ENGINE_DOCUMENT_INTELLIGENCE, 0 # Embedded images are images, so they follow the image side of the admin's configured mode. admin_extraction_mode = get_effective_document_intelligence_pdf_image_extraction_mode(settings) @@ -368,6 +375,7 @@ def _build_office_embedded_image_chunks( chunks = [] analyzed_count = 0 temp_image_dir = None + total_body_words = 0 try: temp_image_dir = tempfile.mkdtemp(prefix='office_images_') @@ -379,6 +387,7 @@ def _build_office_embedded_image_chunks( ) candidate_count = image_diagnostics.get('candidates', 0) + total_body_words = image_diagnostics.get('total_body_words', 0) if not embedded_images: # Say so explicitly. Otherwise "no images in this file" and "images found but all # skipped" look identical in the workspace log, which is the common confusion. @@ -404,7 +413,7 @@ def _build_office_embedded_image_chunks( office_embedded_image_count=0, office_embedded_image_candidates=0, ) - return [], 0, extraction_engine + return [], 0, extraction_engine, total_body_words engine_label = ( "Content Understanding" @@ -456,8 +465,11 @@ def _build_office_embedded_image_chunks( f"{embedded_image.get('name')}{location_label}" ) chunks.append({ - 'page_number': starting_page_number + len(chunks), 'content': f"{heading}\n\n" + "\n\n".join(body_parts), + 'slide_number': embedded_image.get('slide_number'), + 'word_offset': embedded_image.get('word_offset'), + 'position_known': bool(embedded_image.get('position_known')), + 'name': embedded_image.get('name'), }) analyzed_count += 1 @@ -481,7 +493,140 @@ def _build_office_embedded_image_chunks( if temp_image_dir and os.path.isdir(temp_image_dir): shutil.rmtree(temp_image_dir, ignore_errors=True) - return chunks, analyzed_count, extraction_engine + return chunks, analyzed_count, extraction_engine, total_body_words + + +def _resolve_embedded_image_chunk_index(image_block, chunks, total_body_words): + """Return the index of the chunk an embedded image belongs to. + + PowerPoint images carry a slide number, which maps onto the chunk covering that slide. Word + images carry a word offset, which is mapped proportionally rather than absolutely because the + extractor's word count will not match the raw document body exactly; a proportional mapping + keeps images spread across the document instead of clustering them at the front. Images with no + position at all, which is the case for legacy binary Office formats, anchor to the final chunk + so no page number is invented beyond the end of the document. + """ + if not chunks: + return None + + slide_number = image_block.get('slide_number') + if slide_number: + best_index = 0 + for index, chunk in enumerate(chunks): + try: + page_number = int(chunk.get('page_number') or 0) + except (TypeError, ValueError): + continue + if page_number <= slide_number: + best_index = index + return best_index + + word_offset = image_block.get('word_offset') + if word_offset is not None and total_body_words > 0: + relative_position = float(word_offset) / float(total_body_words) + relative_position = min(max(relative_position, 0.0), 1.0) + index = int(relative_position * len(chunks)) + return min(index, len(chunks) - 1) + + return len(chunks) - 1 + + +def _merge_embedded_images_into_chunks(final_chunks, image_blocks, total_body_words, settings): + """Merge analyzed images into the chunk they came from. + + Chunk ids are derived from the page number, so two chunks sharing a page number overwrite each + other in the search index. Image content is therefore appended to the existing chunk's content + rather than emitted as a second chunk, which is also what keeps a figure searchable alongside + the text it belongs to. + + Returns ``(merged_chunks, merged_count, overflow_blocks)``. + """ + if not image_blocks: + return final_chunks, 0, [] + + merged_chunks = [dict(chunk) for chunk in final_chunks] + if not merged_chunks: + return merged_chunks, 0, list(image_blocks) + + try: + chunk_size_cap = int(get_chunk_size_cap(settings)) + except Exception: + chunk_size_cap = OFFICE_IMAGE_MERGE_FALLBACK_CAP + merged_char_limit = max( + OFFICE_IMAGE_MERGE_MIN_CHAR_LIMIT, + int(chunk_size_cap * OFFICE_IMAGE_MERGE_CHARS_PER_TOKEN * OFFICE_IMAGE_MERGE_UTILIZATION), + ) + + merged_count = 0 + overflow_blocks = [] + + for image_block in image_blocks: + content = str(image_block.get('content') or '').strip() + if not content: + continue + + target_index = _resolve_embedded_image_chunk_index(image_block, merged_chunks, total_body_words) + if target_index is None: + overflow_blocks.append(image_block) + continue + + target_chunk = merged_chunks[target_index] + existing_content = str(target_chunk.get('content') or '') + + # Keep merged chunks under the embedding budget; anything that does not fit spills rather + # than silently producing an oversized chunk. + if len(existing_content) + len(content) + 2 > merged_char_limit: + overflow_blocks.append(image_block) + continue + + target_chunk['content'] = ( + f"{existing_content.rstrip()}\n\n{content}" if existing_content.strip() else content + ) + merged_count += 1 + + return merged_chunks, merged_count, overflow_blocks + + +def _append_overflow_image_chunks(merged_chunks, overflow_blocks): + """Append images that could not fit their origin chunk, numbered past the existing chunks.""" + if not overflow_blocks: + return merged_chunks + + next_page_number = max( + (int(chunk.get('page_number') or 0) for chunk in merged_chunks), + default=0, + ) + 1 + + for offset, image_block in enumerate(overflow_blocks): + content = str(image_block.get('content') or '').strip() + if not content: + continue + merged_chunks.append({ + 'page_number': next_page_number + offset, + 'content': content, + }) + + return merged_chunks + + +def _assert_unique_chunk_page_numbers(chunks, document_id): + """Warn when chunks share a page number, which would overwrite entries in the search index.""" + seen_page_numbers = set() + duplicates = set() + for chunk in chunks: + page_number = chunk.get('page_number') + if page_number in seen_page_numbers: + duplicates.add(page_number) + seen_page_numbers.add(page_number) + + if duplicates: + log_event( + f"[OFFICE_EMBEDDED_IMAGES] Duplicate chunk page numbers for document {document_id}: " + f"{sorted(duplicates)}. Chunk ids are derived from the page number, so these would " + "overwrite each other in the search index.", + level=logging.ERROR, + ) + return not duplicates def _resolve_extraction_engine_for_mode(extraction_mode, settings): @@ -7500,28 +7645,50 @@ def process_di_document(document_id, user_id, temp_file_path, original_filename, # --- Embedded Office image analysis (DOCX/DOC/PPTX/PPT) --- # Neither extraction engine describes figures inside Office files, so embedded images are - # analyzed separately and appended as their own citable chunks. Legacy binary formats are - # included because their pictures are carved from the OLE container by signature. + # analyzed separately and merged back into the chunk they came from. Merging rather than + # appending keeps a figure searchable alongside its surrounding text, and avoids inventing + # page numbers past the end of the document. if is_word or is_ppt: - next_chunk_page_number = max( - (int(chunk.get('page_number') or 0) for chunk in final_chunks_to_save), - default=0, - ) + 1 - - embedded_image_chunks, embedded_image_count, embedded_image_engine = _build_office_embedded_image_chunks( - chunk_path, - settings, - update_callback, - starting_page_number=next_chunk_page_number, + image_blocks, embedded_image_count, embedded_image_engine, embedded_total_words = ( + _build_office_embedded_image_chunks( + chunk_path, + settings, + update_callback, + ) ) - if embedded_image_chunks: - final_chunks_to_save = list(final_chunks_to_save) + embedded_image_chunks + if image_blocks: + final_chunks_to_save, merged_image_count, overflow_image_blocks = ( + _merge_embedded_images_into_chunks( + final_chunks_to_save, + image_blocks, + embedded_total_words, + settings, + ) + ) + final_chunks_to_save = _append_overflow_image_chunks( + final_chunks_to_save, overflow_image_blocks + ) + _assert_unique_chunk_page_numbers(final_chunks_to_save, document_id) + update_callback( number_of_pages=len(final_chunks_to_save), office_embedded_image_count=embedded_image_count, + office_embedded_image_merged=merged_image_count, office_embedded_image_engine=embedded_image_engine, ) + if overflow_image_blocks: + update_callback( + status=( + f"Merged {merged_image_count} embedded image(s) into their source " + f"chunk; {len(overflow_image_blocks)} exceeded the chunk size budget " + "and were appended." + ) + ) + else: + update_callback( + status=f"Merged {merged_image_count} embedded image(s) into their source chunk." + ) # Save Final Chunks to Search Index num_final_chunks = len(final_chunks_to_save) diff --git a/application/single_app/functions_office_media.py b/application/single_app/functions_office_media.py index 72ef5db8d..40e167ee2 100644 --- a/application/single_app/functions_office_media.py +++ b/application/single_app/functions_office_media.py @@ -40,6 +40,8 @@ OFFICE_EMBEDDED_IMAGE_MAX_BYTES = 64 * 1024 * 1024 # Slide relationship parts are small XML documents; anything larger is not worth decompressing. OFFICE_EMBEDDED_RELS_MAX_BYTES = 4 * 1024 * 1024 +# The main document part carries the body text used to locate images in reading order. +OFFICE_DOCUMENT_PART_MAX_BYTES = 64 * 1024 * 1024 # A zip header can understate the uncompressed size, so entries are streamed in bounded chunks. OFFICE_ZIP_READ_CHUNK_BYTES = 256 * 1024 # Caps on how much of a crafted archive is inspected at all. @@ -146,10 +148,9 @@ def _build_pptx_media_slide_map(archive): continue for relationship in rels_root: - target = str(relationship.attrib.get('Target') or '').replace('\\', '/') - if '/media/' not in target: + media_name = _normalize_office_media_target(relationship.attrib.get('Target'), 'ppt/media/') + if not media_name: continue - media_name = 'ppt/media/' + target.rsplit('/media/', 1)[-1] # Keep the first slide that references the image so ordering stays stable. media_slide_map.setdefault(media_name, slide_number) @@ -175,6 +176,7 @@ def _new_diagnostics(): 'analyzed': 0, 'skipped': 0, 'skipped_reasons': {}, + 'total_body_words': 0, } @@ -193,7 +195,9 @@ def extract_office_embedded_images(file_path, output_dir, min_pixels=150, max_im max_images (int): Maximum number of images to extract. Returns: - list: Dicts with ``name``, ``path``, ``width``, ``height``, and ``slide_number`` keys. + list: Dicts describing each extracted image, including ``name``, ``path``, ``width``, + ``height``, ``source_format``, ``rasterized``, ``embedded_text``, and the position hints + ``slide_number`` and ``word_offset`` used to place the image back into its origin chunk. """ extracted_images, _diagnostics = extract_office_embedded_images_with_diagnostics( file_path, @@ -320,6 +324,8 @@ def _extract_from_binary_office_file(file_path, output_dir, min_pixels, max_imag 'width': width, 'height': height, 'slide_number': None, + 'word_offset': None, + 'position_known': False, 'source_format': source_format, 'rasterized': True, 'embedded_text': embedded_text, @@ -328,6 +334,117 @@ def _extract_from_binary_office_file(file_path, output_dir, min_pixels, max_imag return extracted_images +DOCX_MAIN_DOCUMENT_PART = 'word/document.xml' +DOCX_MAIN_DOCUMENT_RELS_PART = 'word/_rels/document.xml.rels' +DOCX_NS_WORDPROCESSING = '{http://schemas.openxmlformats.org/wordprocessingml/2006/main}' +DOCX_NS_DRAWING = '{http://schemas.openxmlformats.org/drawingml/2006/main}' +DOCX_NS_RELATIONSHIPS = '{http://schemas.openxmlformats.org/officeDocument/2006/relationships}' +DOCX_NS_VML = '{urn:schemas-microsoft-com:vml}' + + +def _normalize_office_media_target(target, media_prefix): + """Resolve a relationship target to its package media path. + + Relationship targets are relative to the part that declares them, so Word writes + ``media/image1.emf`` while PowerPoint writes ``../media/image1.png``. Both resolve to the same + ``/media/`` location. Requiring a ``media`` parent segment also rejects unrelated + targets such as hyperlinks. + """ + normalized_target = str(target or '').replace('\\', '/') + segments = [segment for segment in normalized_target.split('/') if segment not in ('', '.', '..')] + if len(segments) < 2 or segments[-2].lower() != 'media': + return '' + return f'{media_prefix}{segments[-1]}' + + +def _build_docx_relationship_targets(archive): + """Map relationship ids in the main document part to their ``word/media`` targets.""" + relationship_targets = {} + + rels_bytes = _read_zip_entry_bounded( + archive, DOCX_MAIN_DOCUMENT_RELS_PART, OFFICE_EMBEDDED_RELS_MAX_BYTES + ) + if rels_bytes is None: + return relationship_targets + + try: + rels_root = defused_fromstring(rels_bytes) + except (DefusedParseError, ValueError): + return relationship_targets + + for relationship in rels_root: + relationship_id = str(relationship.attrib.get('Id') or '') + if not relationship_id: + continue + if str(relationship.attrib.get('TargetMode') or '').lower() == 'external': + continue + media_name = _normalize_office_media_target(relationship.attrib.get('Target'), 'word/media/') + if media_name: + relationship_targets[relationship_id] = media_name + + return relationship_targets + + +def build_docx_image_word_offsets(archive): + """Return ``(offsets, total_body_words)`` describing where each image sits in reading order. + + Word has no fixed pages until it is rendered, but the ingestion pipeline chunks Word content by + word count, so an image's position in the document body maps onto a chunk. Walking + ``document.xml`` in document order and counting the words that precede each image reference + gives that position. The body total is returned as well so callers can place images + proportionally, which stays stable when the extractor's word count differs from the raw body. + """ + image_word_offsets = {} + + document_bytes = _read_zip_entry_bounded( + archive, DOCX_MAIN_DOCUMENT_PART, OFFICE_DOCUMENT_PART_MAX_BYTES + ) + if document_bytes is None: + return image_word_offsets, 0 + + try: + document_root = defused_fromstring(document_bytes) + except (DefusedParseError, ValueError): + return image_word_offsets, 0 + + relationship_targets = _build_docx_relationship_targets(archive) + + word_count = 0 + text_tag = f'{DOCX_NS_WORDPROCESSING}t' + blip_tag = f'{DOCX_NS_DRAWING}blip' + image_data_tag = f'{DOCX_NS_VML}imagedata' + embed_attr = f'{DOCX_NS_RELATIONSHIPS}embed' + id_attr = f'{DOCX_NS_RELATIONSHIPS}id' + + # iter() is a document-order traversal, which is the same order the text is extracted in. + for element in document_root.iter(): + tag = element.tag + if tag == text_tag: + word_count += len(str(element.text or '').split()) + continue + + if not relationship_targets: + continue + + relationship_id = '' + if tag == blip_tag: + relationship_id = str(element.attrib.get(embed_attr) or '') + elif tag == image_data_tag: + relationship_id = str(element.attrib.get(id_attr) or '') + + if not relationship_id: + continue + + media_name = relationship_targets.get(relationship_id) + if not media_name: + continue + + # Keep the first occurrence so a repeated image maps to where it is first seen. + image_word_offsets.setdefault(media_name, word_count) + + return image_word_offsets, word_count + + def extract_office_embedded_images_with_diagnostics(file_path, output_dir, min_pixels=150, max_images=25): """Extract embedded images and report what was skipped and why. @@ -364,6 +481,8 @@ def extract_office_embedded_images_with_diagnostics(file_path, output_dir, min_p return [], diagnostics media_slide_map = _build_pptx_media_slide_map(archive) + media_word_offsets, total_body_words = build_docx_image_word_offsets(archive) + diagnostics['total_body_words'] = total_body_words candidate_names = [ entry_name for entry_name in entry_names @@ -453,6 +572,8 @@ def extract_office_embedded_images_with_diagnostics(file_path, output_dir, min_p 'width': width, 'height': height, 'slide_number': media_slide_map.get(media_name), + 'word_offset': media_word_offsets.get(media_name), + 'position_known': media_name in media_word_offsets or media_name in media_slide_map, 'source_format': source_extension.lstrip('.'), 'rasterized': is_vector, 'embedded_text': embedded_text, diff --git a/docs/explanation/features/CONTENT_UNDERSTANDING_ENHANCED_EXTRACTION.md b/docs/explanation/features/CONTENT_UNDERSTANDING_ENHANCED_EXTRACTION.md index 5e02aa4a0..6b18429d3 100644 --- a/docs/explanation/features/CONTENT_UNDERSTANDING_ENHANCED_EXTRACTION.md +++ b/docs/explanation/features/CONTENT_UNDERSTANDING_ENHANCED_EXTRACTION.md @@ -11,7 +11,7 @@ does not produce. Enhanced extraction always degrades gracefully: when Content Understanding is unavailable or unconfigured, Enhanced automatically uses Document Intelligence `prebuilt-layout` instead. -**Implemented in version: 0.250.221** (EMF/WMF diagram support added in 0.250.223) +**Implemented in version: 0.250.221** (EMF/WMF diagram support added in 0.250.223; figure chunk association fixed in 0.250.228) **Tracking issue:** [#1277](https://github.com/microsoft/simplechat/issues/1277) @@ -50,7 +50,7 @@ flowchart TD K --> L{Active engine} L -->|Enhanced| M[Content Understanding prebuilt-imageSearch] L -->|Standard| N[Document Intelligence] - M --> O[Append as citable chunks] + M --> O[Merge into the chunk the image came from] N --> O ``` @@ -174,8 +174,10 @@ them separately. - Ordering is natural, so `image2.png` precedes `image10.png`. - For PPTX, each image is attributed to the slide that references it via `ppt/slides/_rels/slideN.xml.rels`. -- Each analyzed image becomes its own citable chunk with a heading such as - `### Embedded image 2 of 5: image2.png on slide 3`. +- Each analyzed image is merged into the chunk containing the text it appears with, under a heading + such as `### Embedded image 2 of 5: image2.png on slide 3`, so a figure stays searchable and + citable alongside its surrounding content instead of becoming a separate chunk at the end of the + document. - Legacy `.doc` and `.ppt` files are OLE compound documents rather than zip packages, so their pictures are carved out by metafile signature instead of being enumerated from media parts. The carve validates the record type, signature position, and declared length before accepting a blob, @@ -199,6 +201,31 @@ citation, not a pixel-accurate GDI reimplementation. Text drawn inside a metafile is also recovered and attached to the chunk, so figure labels such as service and resource names stay searchable even when the vision engine returns no description. +### Where a figure ends up + +Figures, equations, and tables stay in the chunk containing the text they appear with. Chunk ids are +derived from the page number (`{document_id}_{page_number}`), so two chunks sharing a page number +would overwrite each other in the search index; image content is therefore merged into the existing +chunk rather than emitted as an additional one. + +| Source | How placement is resolved | +| --- | --- | +| PDF via Content Understanding | The service reports a span for each figure, which is matched against the per-page spans. Already page-accurate. | +| PDF via Document Intelligence Layout | Tables and figures are inlined into that page's markdown by the service. | +| Equations | Returned inline in the page markdown by both engines, so they inherit the right page. | +| PPTX | The slide that references the image, mapped onto the chunk covering that slide. | +| DOCX | The image's position in reading order, taken from `word/document.xml`, mapped proportionally onto the word-count chunks. | +| Legacy `.doc` / `.ppt` | No position is recoverable from a carved metafile, so the image anchors to the final chunk rather than creating a page beyond the document. | + +Word has no fixed pages until it is rendered, so DOCX placement is a best-effort mapping onto the +word-count chunks rather than an exact paragraph match. Position is mapped proportionally because the +extractor's word count will not match the raw document body exactly, and an absolute offset would +drift and cluster every image toward the front of the document. + +Merged chunks are held under a size budget derived from the chunk size cap. In the rare case where a +figure-dense chunk would exceed it, the remaining images for that chunk are appended instead, which +keeps chunks from growing unbounded. + ### Confirming that embedded images were processed Processing reports counts rather than staying silent, because a document whose images were all diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 47e9201c3..c24f19320 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,18 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](/explanation/features/) and [Fixes by Version](/explanation/fixes/). +### **(v0.250.228)** + +#### Bug Fixes + +* **Figures Now Stay in the Chunk They Came From** + * Images extracted from Word and PowerPoint files were appended as extra chunks at the end of the document, with page numbers continuing past the real content. A figure on page 5 of a 15-page document became chunk 16, so a search hit on the figure lost its surrounding text and citations pointed at a page that did not exist. + * Embedded images are now merged into the chunk containing the text they appear with. PowerPoint images follow the slide that references them; Word images are placed by their position in reading order; and legacy `.doc` and `.ppt` images, which carry no recoverable position, anchor to the final chunk instead of creating a page beyond the document. + * Merging rather than adding a chunk also removes a latent indexing hazard: chunk ids are derived from the page number, so a second chunk sharing a page number would have overwritten the first in the search index. + * PDFs were already correct — Content Understanding attributes each figure to its page by span, and Document Intelligence Layout inlines tables and figures into the page markdown. That behavior is unchanged and now covered by a regression test. + * **Existing documents keep their current chunks until they are extracted again.** Use *Change Extraction* or re-upload to pick up the new placement. + * (Ref: #1277, `functions_documents.py`, `functions_office_media.py`, figure chunk association) + ### **(v0.250.227)** #### Bug Fixes diff --git a/functional_tests/test_figure_chunk_association.py b/functional_tests/test_figure_chunk_association.py new file mode 100644 index 000000000..86343dba8 --- /dev/null +++ b/functional_tests/test_figure_chunk_association.py @@ -0,0 +1,465 @@ +#!/usr/bin/env python3 +# test_figure_chunk_association.py +""" +Functional test for keeping figures in the chunk they came from. +Version: 0.250.228 +Implemented in: 0.250.228 + +This test ensures embedded Office images are merged into the chunk containing their surrounding +text instead of being appended as extra chunks past the end of the document, that no two chunks +share a page number (chunk ids are derived from it, so duplicates overwrite each other in the +search index), and that the Content Understanding path keeps attributing figures to their origin +page. +""" + +import ast +import logging +import os +import sys +import tempfile +import zipfile +from io import BytesIO +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[1] +APP_ROOT = REPO_ROOT / "application" / "single_app" + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +sys.path.insert(0, str(APP_ROOT)) + +from PIL import Image # noqa: E402 + +from functions_office_media import build_docx_image_word_offsets # noqa: E402 +from test_support.versioning import assert_app_version_at_least # noqa: E402 + + +WORDPROCESSING_NS = "http://schemas.openxmlformats.org/wordprocessingml/2006/main" +DRAWING_NS = "http://schemas.openxmlformats.org/drawingml/2006/main" +RELATIONSHIP_NS = "http://schemas.openxmlformats.org/officeDocument/2006/relationships" + + +def load_document_functions(function_names): + """Exec selected pure functions from functions_documents.py in an isolated namespace. + + functions_documents imports the full Azure config at module load, so the placement and merge + helpers are extracted and run against the real repository source instead. + """ + source = (APP_ROOT / "functions_documents.py").read_text(encoding="utf-8") + tree = ast.parse(source) + + namespace = { + "logging": logging, + "log_event": lambda *args, **kwargs: None, + "get_chunk_size_cap": lambda settings=None: 16384, + "OFFICE_IMAGE_MERGE_CHARS_PER_TOKEN": 4, + "OFFICE_IMAGE_MERGE_UTILIZATION": 0.9, + "OFFICE_IMAGE_MERGE_FALLBACK_CAP": 16384, + "OFFICE_IMAGE_MERGE_MIN_CHAR_LIMIT": 4000, + } + + wanted = set(function_names) + found = set() + for node in tree.body: + if isinstance(node, ast.FunctionDef) and node.name in wanted: + exec(compile(ast.Module(body=[node], type_ignores=[]), "functions_documents.py", "exec"), namespace) + found.add(node.name) + + missing = wanted - found + if missing: + raise AssertionError(f"Could not locate functions in source: {sorted(missing)}") + return namespace + + +def build_png_bytes(width, height, color): + """Build PNG bytes large enough to clear the minimum-size filter.""" + buffer = BytesIO() + image = Image.new("RGB", (width, height), color) + for x in range(0, width, 3): + for y in range(0, height, 3): + image.putpixel((x, y), ((x * 7) % 256, (y * 11) % 256, ((x + y) * 13) % 256)) + image.save(buffer, format="PNG") + return buffer.getvalue() + + +def build_docx_with_images_at(paragraph_indexes, total_paragraphs=60, words_per_paragraph=20): + """Build a DOCX whose images sit at known paragraph positions. Returns (path, planted_offsets).""" + body_parts = [] + planted_offsets = {} + + for paragraph_index in range(total_paragraphs): + for image_number, target_index in enumerate(paragraph_indexes, start=1): + if paragraph_index == target_index: + relationship_id = f"rId{100 + image_number}" + body_parts.append( + f'' + f'' + f'' + f'' + f'' + f'' + ) + planted_offsets[f"image{image_number}.png"] = paragraph_index * words_per_paragraph + + words = " ".join(f"w{paragraph_index}x{word_index}" for word_index in range(words_per_paragraph)) + body_parts.append(f'{words}') + + document_xml = ( + '' + f'' + '' + "".join(body_parts) + '' + ) + + relationship_entries = "".join( + f'' + for number in range(1, len(paragraph_indexes) + 1) + ) + rels_xml = ( + '' + '' + + relationship_entries + + '' + ) + + handle = tempfile.NamedTemporaryFile(suffix=".docx", delete=False) + handle.close() + with zipfile.ZipFile(handle.name, "w") as archive: + archive.writestr("[Content_Types].xml", "") + archive.writestr("word/document.xml", document_xml) + archive.writestr("word/_rels/document.xml.rels", rels_xml) + for number in range(1, len(paragraph_indexes) + 1): + archive.writestr(f"word/media/image{number}.png", build_png_bytes(400, 300, (40 * number % 256, 90, 160))) + + return handle.name, planted_offsets + + +def test_docx_image_positions_are_detected(): + """Word images must resolve to the word offset where they appear in reading order.""" + print("Testing DOCX image position detection...") + + docx_path, planted = build_docx_with_images_at([6, 30, 54]) + try: + with zipfile.ZipFile(docx_path) as archive: + offsets, total_words = build_docx_image_word_offsets(archive) + finally: + os.remove(docx_path) + + if total_words != 1200: + raise AssertionError(f"Expected 1200 body words, got {total_words}") + + for name, expected_offset in planted.items(): + actual = offsets.get(f"word/media/{name}") + if actual != expected_offset: + raise AssertionError(f"{name}: expected offset {expected_offset}, got {actual}") + + print("DOCX position detection test passed!") + return True + + +def test_docx_images_merge_into_their_origin_chunk(): + """An image in the middle of a document must land in the middle chunk, not past the end.""" + print("Testing DOCX image chunk placement...") + + namespace = load_document_functions([ + "_resolve_embedded_image_chunk_index", + "_merge_embedded_images_into_chunks", + "_append_overflow_image_chunks", + ]) + merge = namespace["_merge_embedded_images_into_chunks"] + append_overflow = namespace["_append_overflow_image_chunks"] + + docx_path, _planted = build_docx_with_images_at([6, 30, 54]) + try: + with zipfile.ZipFile(docx_path) as archive: + offsets, total_words = build_docx_image_word_offsets(archive) + finally: + os.remove(docx_path) + + chunk_count = 3 + chunks = [{"page_number": index + 1, "content": f"body text {index + 1}"} for index in range(chunk_count)] + + image_blocks = [ + { + "content": f"### Embedded image: image{number}.png\n\nA described figure.", + "word_offset": offsets[f"word/media/image{number}.png"], + "slide_number": None, + "position_known": True, + } + for number in (1, 2, 3) + ] + + merged, merged_count, overflow = merge(chunks, image_blocks, total_words, {}) + merged = append_overflow(merged, overflow) + + if merged_count != 3: + raise AssertionError(f"Expected 3 merged images, got {merged_count}") + if overflow: + raise AssertionError(f"No image should have overflowed, got {len(overflow)}") + + # The core regression: no chunk beyond the document's real chunk count. + if len(merged) != chunk_count: + raise AssertionError(f"Expected {chunk_count} chunks after merge, got {len(merged)}") + if max(chunk["page_number"] for chunk in merged) != chunk_count: + raise AssertionError("Merging must not create page numbers past the end of the document.") + + # Each image belongs with the text it appeared next to. + for index, number in enumerate((1, 2, 3)): + if f"image{number}.png" not in merged[index]["content"]: + raise AssertionError( + f"image{number}.png should be in chunk {index + 1}: {merged[index]['content'][:120]!r}" + ) + if not merged[index]["content"].startswith("body text"): + raise AssertionError("Original chunk text must be preserved ahead of the image content.") + + print("DOCX chunk placement test passed!") + return True + + +def test_pptx_images_map_to_their_slide_chunk(): + """PowerPoint images follow their slide, including when several slides share a chunk.""" + print("Testing PPTX slide chunk placement...") + + namespace = load_document_functions(["_resolve_embedded_image_chunk_index"]) + resolve = namespace["_resolve_embedded_image_chunk_index"] + + # One slide per chunk, the default. + per_slide_chunks = [{"page_number": number, "content": f"slide {number}"} for number in range(1, 6)] + index = resolve({"slide_number": 3, "word_offset": None}, per_slide_chunks, 0) + if per_slide_chunks[index]["page_number"] != 3: + raise AssertionError(f"Slide 3 should map to chunk 3, got {per_slide_chunks[index]['page_number']}") + + # Grouped slides: chunks start at slides 1, 3 and 5. + grouped_chunks = [{"page_number": number, "content": f"slides {number}+"} for number in (1, 3, 5)] + for slide_number, expected_page in ((1, 1), (2, 1), (3, 3), (4, 3), (5, 5), (6, 5)): + index = resolve({"slide_number": slide_number, "word_offset": None}, grouped_chunks, 0) + actual_page = grouped_chunks[index]["page_number"] + if actual_page != expected_page: + raise AssertionError( + f"Slide {slide_number} should map to chunk {expected_page}, got {actual_page}" + ) + + print("PPTX slide placement test passed!") + return True + + +def test_positionless_images_anchor_to_the_last_chunk(): + """Legacy binary Office files carry no position, so images must not invent a trailing page.""" + print("Testing positionless image fallback...") + + namespace = load_document_functions([ + "_resolve_embedded_image_chunk_index", + "_merge_embedded_images_into_chunks", + "_append_overflow_image_chunks", + ]) + merge = namespace["_merge_embedded_images_into_chunks"] + append_overflow = namespace["_append_overflow_image_chunks"] + + chunks = [{"page_number": number, "content": f"body {number}"} for number in (1, 2, 3)] + image_blocks = [{ + "content": "### Embedded image: carved.emf\n\nA described figure.", + "word_offset": None, + "slide_number": None, + "position_known": False, + }] + + merged, merged_count, overflow = merge(chunks, image_blocks, 0, {}) + merged = append_overflow(merged, overflow) + + if merged_count != 1: + raise AssertionError(f"Expected the image to merge, got {merged_count}") + if len(merged) != 3: + raise AssertionError(f"Expected no new chunk, got {len(merged)} chunks") + if "carved.emf" not in merged[-1]["content"]: + raise AssertionError("A positionless image should anchor to the final chunk.") + + print("Positionless fallback test passed!") + return True + + +def test_oversized_image_content_spills_instead_of_bloating_a_chunk(): + """Merging must respect a size budget so a chunk cannot grow unbounded.""" + print("Testing chunk size budget...") + + namespace = load_document_functions([ + "_resolve_embedded_image_chunk_index", + "_merge_embedded_images_into_chunks", + "_append_overflow_image_chunks", + ]) + merge = namespace["_merge_embedded_images_into_chunks"] + append_overflow = namespace["_append_overflow_image_chunks"] + + chunks = [{"page_number": 1, "content": "body text"}] + huge_content = "x" * 200000 + image_blocks = [{ + "content": huge_content, + "word_offset": 0, + "slide_number": None, + "position_known": True, + }] + + merged, merged_count, overflow = merge(chunks, image_blocks, 100, {}) + merged = append_overflow(merged, overflow) + + if merged_count != 0: + raise AssertionError("Oversized image content must not be merged into the chunk.") + if len(overflow) != 1: + raise AssertionError(f"Oversized content should overflow, got {len(overflow)}") + if len(merged) != 2 or merged[-1]["page_number"] != 2: + raise AssertionError(f"Overflow should append exactly one trailing chunk: {merged}") + if merged[0]["content"] != "body text": + raise AssertionError("The origin chunk must be left untouched when content does not fit.") + + print("Chunk size budget test passed!") + return True + + +def test_merged_chunks_never_share_a_page_number(): + """Chunk ids derive from page numbers, so duplicates would overwrite each other in the index.""" + print("Testing chunk page number uniqueness...") + + namespace = load_document_functions([ + "_resolve_embedded_image_chunk_index", + "_merge_embedded_images_into_chunks", + "_append_overflow_image_chunks", + "_assert_unique_chunk_page_numbers", + ]) + merge = namespace["_merge_embedded_images_into_chunks"] + append_overflow = namespace["_append_overflow_image_chunks"] + assert_unique = namespace["_assert_unique_chunk_page_numbers"] + + chunks = [{"page_number": number, "content": f"body {number}"} for number in range(1, 5)] + image_blocks = [ + { + "content": f"### Embedded image {number}\n\nDescription.", + "word_offset": number * 100, + "slide_number": None, + "position_known": True, + } + for number in range(1, 8) + ] + + merged, _merged_count, overflow = merge(chunks, image_blocks, 800, {}) + merged = append_overflow(merged, overflow) + + page_numbers = [chunk["page_number"] for chunk in merged] + if len(page_numbers) != len(set(page_numbers)): + raise AssertionError(f"Duplicate chunk page numbers would collide in the index: {page_numbers}") + if not assert_unique(merged, "doc-1"): + raise AssertionError("Uniqueness assertion reported duplicates.") + + print("Chunk uniqueness test passed!") + return True + + +def test_content_understanding_figures_stay_on_their_page(): + """The PDF path already associates figures by span; keep that behavior locked down.""" + print("Testing Content Understanding page association...") + + from test_content_understanding_extraction_engine import load_content_understanding_module + + content_understanding, _ = load_content_understanding_module() + + page_one = "# Intro\n\nOpening text.\n\n" + page_two = "## Architecture\n\nThe diagram shows the flow.\n\n" + page_three = "## Appendix\n\nClosing notes.\n" + markdown = page_one + page_two + page_three + + result = { + "contents": [ + { + "kind": "document", + "markdown": markdown, + "startPageNumber": 1, + "pages": [ + {"pageNumber": 1, "spans": [{"offset": 0, "length": len(page_one)}]}, + {"pageNumber": 2, "spans": [{"offset": len(page_one), "length": len(page_two)}]}, + { + "pageNumber": 3, + "spans": [{"offset": len(page_one) + len(page_two), "length": len(page_three)}], + }, + ], + "figures": [ + { + "id": "fig-1", + "kind": "chart", + "description": "A bar chart of quarterly revenue by region.", + "span": {"offset": len(page_one) + 10, "length": 8}, + } + ], + } + ] + } + + pages = content_understanding.build_pages_from_content_understanding_result(result) + page_numbers = [page["page_number"] for page in pages] + if page_numbers != [1, 2, 3]: + raise AssertionError(f"Content Understanding must not add pages: {page_numbers}") + + carrying = [page["page_number"] for page in pages if "bar chart of quarterly revenue" in page["content"]] + if carrying != [2]: + raise AssertionError(f"Figure should stay on page 2, found on {carrying}") + + print("Content Understanding association test passed!") + return True + + +def test_pipeline_merges_instead_of_appending(): + """The ingestion pipeline must no longer append embedded images past the document.""" + print("Testing pipeline merge wiring...") + + documents = (APP_ROOT / "functions_documents.py").read_text(encoding="utf-8") + + for removed_marker in ("starting_page_number", "next_chunk_page_number"): + if removed_marker in documents: + raise AssertionError(f"Append-at-end logic still present: {removed_marker}") + + for required_marker in ( + "def _resolve_embedded_image_chunk_index", + "def _merge_embedded_images_into_chunks", + "def _append_overflow_image_chunks", + "def _assert_unique_chunk_page_numbers", + "_merge_embedded_images_into_chunks(", + "office_embedded_image_merged", + ): + if required_marker not in documents: + raise AssertionError(f"Missing merge wiring: {required_marker}") + + print("Pipeline merge wiring test passed!") + return True + + +def test_version_is_at_least_implementation_version(): + """The app version must be at or beyond the version this fix shipped in.""" + print("Testing application version...") + assert_app_version_at_least("0.250.228") + print("Version test passed!") + return True + + +if __name__ == "__main__": + tests = [ + test_docx_image_positions_are_detected, + test_docx_images_merge_into_their_origin_chunk, + test_pptx_images_map_to_their_slide_chunk, + test_positionless_images_anchor_to_the_last_chunk, + test_oversized_image_content_spills_instead_of_bloating_a_chunk, + test_merged_chunks_never_share_a_page_number, + test_content_understanding_figures_stay_on_their_page, + test_pipeline_merges_instead_of_appending, + test_version_is_at_least_implementation_version, + ] + + results = [] + for test in tests: + print(f"\nRunning {test.__name__}...") + try: + results.append(test()) + except Exception as error: + print(f"Test failed: {error}") + import traceback + + traceback.print_exc() + results.append(False) + + print(f"\nResults: {sum(1 for result in results if result)}/{len(results)} tests passed") + sys.exit(0 if all(results) else 1)