0.9.0: extraction correctness pass — images, page tags, shapes, cells, spreadsheets, PDF quality - #20
Merged
Merged
Conversation
…xtraction Thirteen call sites addressed a service API that does not exist. Every one of them sits inside an `except Exception` branch (so one bad image cannot abort a document), so the failures degraded silently: * `ImageService.save_and_tag()` takes `image_data`; eight handlers passed `image_bytes=` (doc, docx, html, ppt, pptx, xls, xlsx×2) and PDF passed `page_num=`/`image_index=`. Result: no image tag anywhere in the output, so the OCR pass never saw the image either — unrecoverable downstream. * `ImageService.save_image()` does not exist at all; hwp, hwpx (×2) and the PDF block-image engine called it. * `TagService` exposes `create_page_tag`/`create_slide_tag`/`create_sheet_tag`; handlers called `make_*_tag` (docx, ppt, pptx, xls, xlsx) or `page_tag` (pdf_plus, pdf_default) and fell back to a hardcoded literal. pdf_plus emitted `[Page N]` and pptx `[Slide:N]` — neither matches the configured prefix, so `PageChunkingStrategy.can_handle()` never fired and every chunk came back with `page_number=None`. DOCX emitted no page marker at all. The suite stayed green throughout because the service test doubles were plain `MagicMock`s that accept any attribute and any keyword — the mocks had even been written against the wrong names. They are now `create_autospec` of the real classes, so a drifting call site fails in the unit tests. Also: * `tests/unit/services/test_service_contracts.py` — AST walk validating every service call site against `inspect.signature`, so this class of bug cannot come back silently. * `tests/integration/test_real_service_extraction.py` — extraction through the *real* services, asserting image tags, page/slide/sheet markers, chunk `page_number`, and that a custom tag configuration actually reaches output. Five of these fail on the parent commit. * `metadata_enricher` treats the injected document-metadata block as front matter rather than page content, so an opening chunk carrying it is attributed to the first page it declares; a chunk that opens with real text from the previous page keeps that page, as before. * pytest `asyncio_mode = "auto"` — without it the async tests error out on a fresh checkout. Verified on real documents: a Korean DOCX went from 0 to 2 image tags and 0 to 6 page markers, and its chunks from page_number=None to fully attributed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… skipping them Three body-level containers were walked past, each taking its content with it. **`mc:AlternateContent`** — Word writes every modern shape twice: a DrawingML rendering under `mc:Choice` and an equivalent VML one under `mc:Fallback`. The paragraph walker only knew `w:r` and `w:hyperlink`, so text boxes, shape captions and grouped-shape labels produced nothing at all — and the picture inside the shape was lost with them. `resolve_alternate_content()` now commits to one branch (Choice, else Fallback) and the resolved children go through the same run handling as anything written directly in the run, so the content is read exactly once. Shape text itself lives in `w:txbxContent`, which no descriptor covered; `_extract_shape_text()` reads it for both renderings, and `_collect_text()` resolves nested compatibility branches so a shape inside a shape still contributes its text once. **`w:sdt`** — a content control is a wrapper, not content. Tables of contents, bibliographies, cover-page fields and any user-inserted control keep their real paragraphs and tables under `w:sdtContent`, so a walk matching only `w:p`/`w:tbl` dropped all of it. `iter_block_elements()` unwraps them recursively, and the paragraph walker does the same for inline controls. **Headers and footers** — these were read through `part.paragraphs`, which sees paragraphs and nothing else. A header table is the usual place a document keeps its number, revision and classification, and all of it was discarded. Both parts now go through the same block iterator as the body, so tables, content controls and shapes come through too. Checked but deliberately not copied: doc2chunk special-cases a first section whose header reports `is_linked_to_previous`. Writing to a header clears that flag, so when it is set there is genuinely no own header part to read; the special case would emit an inherited header twice. Verified with python-docx before dropping it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…y paragraph
`parse_hwpx_section()` found its paragraphs with `root.findall(".//hp:p")` —
every paragraph anywhere in the subtree. But a table cell, a shape's text box
and a header part all store their content as `hp:p` too, so each one was
reported a second time as a loose body paragraph. Table content came out twice
(once inside the rendered HTML, once as bare lines after it), and shape and
header text landed at the end of the section instead of where they belong.
The walk now follows containment: `_iter_body_paragraphs()` descends through
structural wrappers but stops at any container that renders its own content,
so a paragraph is reached by exactly one renderer. Everything a paragraph can
hold routes through a single `_process_node()`, which makes the nesting cases
behave consistently — a table inside a shape, a shape inside a control, a
control inside a run.
That restructuring is also what makes the missing content reachable:
* **Shape text** (`hp:drawText` → `hp:subList`) is rendered where the shape
sits. Known shape tags are recursed into so grouped shapes contribute each
member once; an unrecognised element is still read when it carries a text
box, so a shape type from a newer Hangul release is not lost.
* **Controls** previously matched `hc:pic`/`hp:pic` and nothing else, so a
table, chart or shape anchored in a control was dropped entirely.
* **Headers, footers, footnotes and endnotes** are page furniture, not running
text; splicing them inline puts a page header in the middle of a sentence.
They are collected into `HwpxSupplementary` and rendered as `[Headers]` /
`[Footers]` / `[Footnotes]` blocks, matching the DOCX handler. A caller that
passes no collector still gets them inline rather than losing them.
Correction to an earlier reading of this code: the table renderer was never
broken — cells are placed from `hp:cellAddr`, and a fixture without those
attributes collapses every cell onto (0,0). Real HWPX files always carry them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`<table[^>]*>.*?</table>` is non-greedy, so on a nested table it stops at the inner `</table>`. The region it reported ended in the middle of the outer table, and the chunker treated the remainder as ordinary prose — free to split anywhere, which produced chunks with unbalanced table markup and an outer table whose tail rows drifted into a neighbouring chunk. Nesting is not an edge case here: a merged cell holding a sub-table is how HWP, DOCX and PDF layouts express a form, and the extractors render it as nested `<table>` markup. `find_html_tables()` counts depth in a single pass and returns the outermost regions. Malformed markup is ignored rather than guessed at: an unclosed `<table>` yields no region, so it cannot swallow the rest of the document, and a stray `</table>` is skipped. `HTML_TABLE_PATTERN` stays — it is still the right tool for "does this text contain a table at all" — with a note pointing at the new function for region work. doc2chunk solves the same problem by recording every character offset of every table in a `set`, which is O(document size) in memory and time; the depth counter needs neither. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Splitting a table at a row boundary used to clamp `rowspan` values so the markup stayed well formed, and stop there. The cell on the other side of the cut — written in an earlier row, spanning into this chunk — was simply absent: its text gone, and every row under it one column short. For the usual shape, a left-hand category column spanning its group of rows, that is the single most useful cell in the table. Measured on a three-group table split into six chunks, three of the six lost their category label entirely. `compute_carried_cells()` walks the data rows once and records, per row, the spans that cover it but are written above it. `reissue_carried_cells()` writes those cells back into the row that opens a chunk, at the column they occupy, and the existing clamp then trims the span to the rows the chunk holds. Column positions come from an occupancy grid rather than a running count of the cells present in the row: a row under a span omits the covered cell, so counting only what is written puts every later cell one column too far left. doc2chunk's equivalent counts written cells, which lands the carry-over correctly only when the span sits in the first column — the case its author had in front of them. A mid-table span or a preceding `colspan` misplaces it, and both are covered by tests here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Report portals — Korean government sites in particular — serve an HTML table with a `.xls` or `.xlsx` name. Excel renders them, so users treat them as spreadsheets, but xlrd and openpyxl reject them and the document failed to convert at all: `ConversionError`, nothing extracted. Detection is a signature test rather than a guess. A real XLS opens with the OLE compound-file magic and a real XLSX with the ZIP local-file header, so neither can open with markup. Excel's own SpreadsheetML is the one ambiguous case — it also opens with `<?xml` — and is separated by requiring an actual `<html>` element to follow. Both Excel handlers then delegate to the HTML handler through the existing `_check_delegation()` hook, so these files get the structured output the HTML pipeline already produces. doc2chunk instead reimplements HTML table parsing inside the Excel handler (~280 lines of grid building, colspan handling and markdown rendering that the HTML handler already has); delegating keeps one implementation. `merge_split_tables()` repairs the shape these exporters actually emit: the column headings as one `<table>` and the data as a second one right after it, so the heading row can be frozen while the body scrolls. Read literally the data table has no column names, and chunking it strips every value of its label. The merge is deliberately narrow — the first table must contain header cells and nothing else, the second data cells and nothing else, they must be adjacent siblings with nothing between them, and they must agree on their column count. doc2chunk checks only the cell kinds, which would fuse a genuinely unrelated pair. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…locks intact Three defects in worksheet region handling, two of which lose data outright. **Silent truncation.** Both handlers scanned a fixed 1000×100 cell window. xlrd's `nrows`/`ncols` and openpyxl's `max_row`/`max_column` are the sheet's real extent, so anything past the window was dropped with nothing in the output to say so — a 1,500-row export lost a third of its rows. The scan now follows the reported extent; the constants become a sanity bound against a corrupt dimension record (200k rows / Excel's own column maximum) and clipping logs a warning. XLSX switches to `iter_rows()`, which streams the used range instead of materialising a cell object per coordinate, so the wider scan is not slower in practice. **Merged columns split their own table.** Only the anchor cell of a merged range holds a value; the rest read as empty. A table with a merged category column down its left edge therefore broke into several "unrelated" blocks at every run of empty rows, and every block after the first lost the column entirely. Region detection now treats a merged range as occupied along its whole extent, so the table stays together. On top of that, merges are clipped to the region they are rendered in: a merge anchored outside contributes its value at the first cell it covers (previously that cell was skipped, losing the label and leaving the row a column short), and a merge running past the region no longer leaves a `rowspan` pointing at rows the table does not have. **One-cell "tables".** A stray note in an otherwise empty part of a sheet was detected as a region and rendered as a one-column table — a header separator around a single value which, because tables are protected from splitting, then occupied a chunk of its own. Regions smaller than 2×2 render as plain text instead; the values are kept, so nothing is dropped. The threshold matches the minimum the DOCX, HWPX and PDF table detectors already enforce. Checked but not ported: doc2chunk also counts a bordered empty cell as a layout boundary. In this codebase the layout bounds come from cells that hold values, and every value is inside those bounds by construction, so the change would alter region shape without recovering any content. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…gment A sheet is split into segments and each chunk carries the document metadata and sheet marker so it can stand alone. Emitting a chunk per segment made that prefix the bulk of every chunk: six small tables against a 1000-character budget produced eight chunks averaging 97 characters, roughly 60% of each one the same boilerplate. Downstream that is eight embeddings and eight retrieval candidates where one would do, each carrying almost no distinguishing content. Segments that fit are now buffered and emitted together. The budget is measured on segment bodies because the prefix is written once per chunk, not once per segment. A sheet boundary always flushes, so no chunk claims two sheet markers, and an oversized segment keeps its previous handling (tables split by row, charts stay whole, prose is recursively split). Image tags and textbox blocks are no longer segments of their own. They are short and belong with the prose around them; treating each as a boundary handed a lone `[Image:…]` tag a chunk to itself. Protected-region handling already prevents them from being split. Two related doc2chunk changes were examined and deliberately not taken: * Merging page-marker-only chunks into the following chunk. Such a chunk means a page had no content of its own; attaching its marker to the next chunk makes the enricher report the *next* page's content under the empty page's number. Measured on a document with a blank page 2, attribution is already correct (1, 3, 4) and the change would break it. * Merging any chunk under 10% of the budget into its neighbour, unconditionally. Across the real documents in this repo only 0.9% of chunks (3 of 329) are that small, and each is a genuinely short section; doc2chunk's version has no size check, so chained merges can produce chunks several times the budget. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…er it exists The quality-aware fallback was reached only when structured extraction returned nothing at all (`if not text_found`). A page whose font mapping failed still returns plenty of characters — they are simply the wrong ones — so the worst pages in a corpus were exactly the ones that never triggered it. Quality is now measured up front and drives the decision. Two signals were added to that measurement: * **CJK Compatibility density.** A Word export whose font could not be mapped turns punctuation into squared unit symbols: brackets as ㏙/㏚, a range dash as ㏊. Density is the signal; the characters are never substituted. The block holds legitimate unit abbreviations (㎏, ㎞, ㎡, ㏈) that Korean technical writing uses constantly — a test here pins that a page using them normally is left alone. doc2chunk instead ships a substitution table, and six of its nine mappings target CJK Extension A, which is ordinary Hanja: rewriting U+3711 (㜑) as an arrow corrupts any document that genuinely uses it. * **Fragmentation.** Some producers, Word text boxes and vertical-text frames in particular, export each glyph as its own text object, and the extractor has no grounds to join them: `현재 시장` arrives as five lines of one character. `FragmentedTextReconstructor` rebuilds the lines from the glyph positions the PDF already carries — no guessing, and unlike OCR it cannot be worse than the original. The detector is deliberately stricter than doc2chunk's: it asks what share of lines hold almost nothing, where doc2chunk asks whether the average line is under fifteen characters. A bulleted list, a table of contents and a column of figures all fail that average and are perfectly intact; tests cover each. Two containment fixes come with it. A whole-page fallback re-reads everything including the tables the table renderer is already emitting, so it is skipped entirely on a page that has tables. And table regions are now excluded per line as well as per block: the extractor groups blocks by column, not by table, so a block can straddle a table edge — dropping it took the prose with it, keeping it repeated the cells. Verified that none of the new triggers fire on the healthy PDFs in the repo. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A chart title is a sequence of text runs, and a run boundary is a formatting change rather than a word boundary — a title with one bold word is split mid-phrase. DOCX and HWPX read only the first run, so "2026년 매출 실적" arrived as "2026"; XLSX read them all but joined with spaces, so the same title arrived as "2026 년 매출 실적". All three embed the identical `c:chart` part, so they now share one reader. It joins runs within a paragraph with nothing between them, joins wrapped title paragraphs with a space, handles a title authored directly under `c:rich` without an `a:p` wrapper, and falls back to the `c:strRef` cache for a title that points at a cell — a form none of the three DOCX/HWPX paths handled at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ch image once **Field codes.** Legacy .doc stores a field as `\x13 instruction \x14 result \x15`. The instruction is markup Word never displays — `PAGE \* MERGEFORMAT`, `HYPERLINK "https://…"`, `REF _Ref12345` — but the cleaner removed only the three control characters, leaving all of it in the body. Every document with page numbers, a table of contents, hyperlinks or cross-references carried that noise into its chunks. `strip_field_codes()` keeps the displayed result and drops the instruction, tracking nesting with a stack so an `IF` field taking fields as arguments resolves correctly. An unterminated field rolls back to where it opened and keeps the remainder verbatim rather than swallowing the rest of the document — doc2chunk's regex loop instead gives up after fifty iterations with no such guard. **OCR duplication.** Image tags were collected with `findall`, so a picture referenced more than once — a logo in a repeated header, a diagram cited from two sections — was converted once per reference. With parallel workers these are not even a cache hit on each other: they are simultaneous calls for the same answer. Conversion now runs once per distinct path; replacement already covered every occurrence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… controls A table cell was read for its text runs and nothing else. Three kinds of content went missing, and one of them was unrecoverable. **Pictures in cells.** Pasting a screenshot into a cell is how scanned figures normally enter a document. The cell rendered empty and — this is the part that matters — no `[Image:…]` tag appeared anywhere in the output, so the OCR pass, which works from the tags in the text, never learned the image existed. DOCX and HWPX now emit the tag at the cell that holds it. Any caption in the same cell is kept alongside rather than replaced: doc2chunk tags only cells that are otherwise empty, which discards the image whenever a cell carries both. **Sub-tables.** Only direct `w:p` children of a cell were visited, so a nested table — the usual way a form expresses a sub-grid — was dropped. Cells now carry it in `TableCell.nested_table`, a field that already existed and that no renderer read. HTML nests it (the chunker's table scanner counts depth, so it stays inside its parent's protected region); Markdown and plain text flatten it into the cell, since neither can nest and losing it is worse. **Content controls in cells** are unwrapped like anywhere else. The OCR side has to hold up its end. Vision models answer a "read this" prompt with whatever structure they see, and the default prompt asks for HTML tables; inserted into a `<td>` that is nested markup the table parser cannot read. Replacement is now position-aware: text landing inside a cell is dissolved into one escaped line, text anywhere else is inserted verbatim, and a conversion that failed leaves its tag in place to be retried. Cost control for decorative icons falls out of the deduplication in the previous commit — a checkbox repeated down a column is one image, so it is one conversion, which is why no pixel-size threshold is needed here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…reading "Does this file contain a forbidden word or a piece of personal data?" needs the words and nothing else, but answering it went through the full pipeline: table reconstruction, image extraction, layout and quality analysis, chart parsing. On a 33-page PDF that is 8.3 seconds to answer a question the text layer alone settles in 42 milliseconds. `DocumentProcessor.extract_text_fast()` returns plain text — no metadata block, no image tags, no chart blocks, no structure. `BaseHandler` supplies a correct default (the normal pipeline with metadata and OCR off) so every format works; the formats whose analysis dominates override it. Measured on real documents: PDF 197×, PPTX 7.0×, XLSX 2.9×, DOCX 3.1×. Two things that only measurement would have caught. The obvious DOCX implementation — python-docx's `paragraphs` and `tables` — runs at *half* the speed of the full pipeline, because its object model rebuilds rows and cells on access; reading `w:t` nodes out of the package XML is what makes it faster, and it picks up headers, footers, notes and text boxes for free. And joining those nodes one line each looked fine until a search for a real phrase came back empty: Word splits a phrase across runs at every formatting change, so the join has to be per paragraph. An image file returns empty rather than running OCR — OCR is the cost this path exists to avoid, and a caller that wants the picture read should ask for `extract_text`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…for failures Two fixes in the shared base, found while adding the engine: * `response.content.strip()` assumes a string. A chat model can answer with a list of content blocks — every OpenAI-compatible server does for multi-part answers, and vLLM does for a single part when `skip_special_tokens` is off. The call raised, and the engine reported a working model as broken. `normalize_response_content()` handles both shapes. * An empty answer produced `[Figure:]`, which replaced the image tag with a placeholder holding nothing — the reference to the image gone, and nothing left to retry from. It is now reported as a failure, so the tag stays. `DeepSeekOCREngine` is not interchangeable with the generic vLLM engine even though both speak the same protocol: DeepSeek-OCR's chat template puts the image before the instruction and the model is trained on its own fixed prompt rather than arbitrary instructions. Sent the generic engine's message shape it answers with nothing at all. Both details are pinned by tests. The server-side requirements from the vLLM recipe are documented on the class, since no client can set them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Records what changed, and just as deliberately what did not: six of doc2chunk's fixes were reviewed and declined (a CJK substitution table that rewrites real Hanja, a fragmentation test that fires on bulleted lists, page-marker merging that misattributes pages, unbounded small-chunk merging, relaxed table validation, and page numbers for formats whose pagination is never computed), and five legacy-binary items were left unported because no `.doc` sample exists here and LibreOffice cannot convert in this environment — merging 1,700 lines of struct parsing that never ran would be worse than the heuristics in place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`ruff format --check contextifier/` is a required CI step and installs the newest ruff, so the files touched in this series (and the Python examples in ARCHITECTURE.md, which newer ruff formats inside Markdown) have to match it. No behaviour change: 807 tests pass on Python 3.12 and on 3.13 with `.[all]`, the same matrix CI uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A correctness pass driven by comparing Contextifier against
xgen_doc2chunk, a fork that stayed on the pre-2.0 architecture and kept fixing real documents. Every item was reproduced here first; the fork's own fix was reviewed rather than copied, and several were rejected. Full rationale is in each commit message and inCHANGELOG.md.Content that never reached the output
save_and_tag(image_bytes=),save_image(),make_page_tag/page_tag). All sat inexcept Exception, so image extraction was dead in 7 handlers and page/slide tagging in 5 —page_numberwasNoneon every chunk. The service mocks had been written against the wrong names; they are nowcreate_autospec, plus an AST contract test.mc:AlternateContent(text boxes/shapes),w:sdtcontent controls, header tables..//hp:pduplicated every table cell and misplaced shape/header text; traversal now follows containment.TableCell.nested_tableis finally rendered..xls/.xlsx.</table>, merged cells lost across chunk boundaries, one chunk per spreadsheet segment.Added:
DocumentProcessor.extract_text_fast()(PDF 197×, PPTX 7×, XLSX 2.9×, DOCX 3.1×),FragmentedTextReconstructor,DeepSeekOCREngine, OCR de-duplication and cell-safe replacement.Declined after review: a CJK substitution table (6 of 9 mappings rewrite real Hanja), an "average line < 15 chars" fragmentation test, forward-merging page-marker chunks (misattributes pages), unbounded small-chunk merging, relaxed PDF table validation,
[Page Number: 1]for DOC/RTF.Not ported (unverifiable here):
.docTDefTable tables / text boxes / PCDT search and.xlsBIFF text boxes / Escher images — no samples and LibreOffice cannot convert in this environment.Measured on real documents (before → after)
Test plan
.[all](CI matrix)ruff check contextifier/andruff format --check contextifier/clean on ruff 0.16.7mainand pass on this branch (verified per item)python -m buildproducescontextifier-0.9.0wheel/sdist containing the new modulesopen_raw()round-trip still intact🤖 Generated with Claude Code