12 KiB
ADR 0005: segment/ output contract needed by chunk/ — structure-preserving, not flattened
Status
Proposed. Contract/schema only — no implementation. segment/models.py,
segment/assembler.py, and segment/io.py are actively owned by a parallel
session on the same checkout at the time of writing; this ADR specifies what
chunk/ needs from segment/'s output precisely enough to implement and
test, but does not touch those files itself. Supersedes part of ADR 0004
(see "Relationship to ADR 0004" below) — ADR 0004's (drug_id, section_key)
chunk-unit-as-leaf assumption is corrected here to (drug_id, section_key)
as a parent, with sentence-window splitting demoted from primary
strategy to fallback.
Context
ADR 0004 designed chunking against segment/models.py's current output:
SectionSpan.text is a single flattened string per section ("\n".join( body_line.strip() for ...)), with all per-line style (Span.bold) and
per-line page position discarded once the string is built (confirmed by
reading assembler.py: body_lines.append(span.text.strip()) keeps only
span.text, nothing else). Review of ADR 0004 surfaced four real problems
that trace back to this flattening, not to the chunking algorithm itself:
- A section is not a single semantic unit.
liều lượng và cách dùngandtương tác thuốcroutinely contain multiple distinct facts (dosing per patient population, dosing per organ-function impairment, multiple separate drug interactions) that a doctor may want to retrieve independently. Measured: an explicit population marker ("Người lớn"/ "Trẻ em"/"Trẻ sơ sinh"/"Suy thận"/"Suy gan" immediately followed by:or.) appears in 303 of 675 monographs (44.9%) that have aliều lượng và cách dùngsection — this is common, not an edge case. - A blind sentence-boundary sliding window (ADR 0004's original
sub-chunking method) can still split two different facts into the same
chunk, or split one fact across two chunks, because it has no way to
know a population/interaction boundary exists — that information exists
in the source (as a bold or otherwise visually distinct sub-heading, per
direct reading of MORPHIN SULFAT/VITAMIN D section text: lines like
"Thuốc uống", "Cách dùng:" render as isolated bold short lines in the
PDF) but is discarded before
chunk/ever sees it. - Tables inside the monograph range are not addressed at all. ADR 0004
implicitly assumed monograph-range sections are prose.
docs/pdf-parsing- outlier-catalog.mditem 19 already documents a real table (dosing by renal function, HSV/CMV columns) inside a monograph body (Foscarnet natri, physical page 698) — flattening a table's rows into newline-joined body text destroys its row/column structure exactly the way outlier item 7 already describes for the appendix's 2D nomogram table. A whole-range survey to size this properly is in progress alongside this ADR (see "Not yet resolved" below). - Provenance is section-level, not chunk-level, because per-line
physical_page/y0(whichSpanalready carries — seeextract/models.py) is discarded at the same flattening point. For a section spanning several physical pages, a sub-chunk built from its final third currently has no way to know its own real page — it can only inherit the whole monograph'ssource_page_range. For medical citations this is not precise enough.
A fifth, independently-found data-quality bug makes precise provenance
even more necessary, not less: the corpus's last-processed monograph
(ZOLPIDEM) is never closed until true end-of-stream, and assembler._classify
calls match_section()/match_section_with_inline_value() on every span
with no in_monograph_range gate (unlike _TextEvent handling, which
does check it). A spurious bold-text match on physical page 1655 — deep in
the back-of-book "Mục lục tra cứu" brand-name index, confirmed by reading
that page directly — overwrote ZOLPIDEM's real tương tác thuốc
SectionSpan with an empty one and corrupted its source_page_range to
[1492, 1655]. This is real content loss (measured: exactly 1 monograph
affected, the last one processed — every other monograph is closed on
schedule by the next monograph title, which is range-gated). Flagged for
the session that owns extract/segment, not fixed here.
Decision
Extend segment/models.py's SectionSpan with a structured, line-level
representation, additive to (not replacing) the existing flat text field
— chunk/ becomes a real, structure-aware consumer instead of re-deriving
structure from a flattened string via ad hoc regex.
New/changed types (segment/models.py)
@dataclass(frozen=True)
class BodyLine:
text: str
physical_page: int
y0: float
bold: bool # Span.bold, preserved instead of discarded
@dataclass
class SectionSpan:
key: str
display_name: str
heading: Heading
text: str # UNCHANGED meaning, kept for
# backward compat (see invariant below)
lines: List[BodyLine] = field(default_factory=list) # NEW
lines carries exactly the per-line signal chunk/ needs to do its own
job (population/subheading detection, precise page provenance) without
segment/ having to know anything about chunking — segment/'s
responsibility stays "detect boundaries and preserve source structure," not
"decide what a retrieval unit is" (Clean Architecture / SoC, per
CLAUDE.md). Specifically, this is deliberately not a is_subheading: bool field computed by segment/ — classifying "is this line a
subheading a chunker should split on" is a chunking-time decision (what
counts as a good split point can vary by strategy/eval results), not a
segmentation-time one. segment/ should stop discarding the raw signal
(bold, y0, physical_page) it already has per span; it should not also
start doing chunk-shaping judgment calls.
Invariants
text == "\n".join(l.text for l in lines).strip()for everySectionSpan, for the lifetime of this contract —linesis a strictly additive refinement, never a divergent second source of truth. Any change to how body text is assembled (e.g. the boilerplate-stripping fix already applied by the other session) must update both fields from the same filtered span list, nottextalone.linesis in reading order, matching the ordertext's lines already implicitly have.- Every
BodyLine.physical_pagesatisfiesdetector.in_monograph_rangefor aSpanon that page — i.e., no line in anySectionSpan.linesmay come from outside the monograph's real printed-page range. This is the ZOLPIDEM bug's exact failure mode stated as an invariant: it was violated (a spurious section event was accepted from a fully out-of-range page precisely because no such check existed for section events, only for body text events). Enforcing this invariant closes that bug as a side effect, but the invariant is stated here as a contract requirement independent of any specific fix implementation. - Every currently-open monograph must be finalized exactly once, at either (a) the next monograph title, or (b) true end-of-stream — with no third path (e.g., a stray out-of-range section match) able to silently mutate an already-"complete" monograph's sections after point (a) would otherwise have applied. (This is a restatement of invariant 3 from the monograph-lifecycle side, not a new requirement.)
Migration impact
segment/io.py(_monograph_to_dict/_monograph_from_dict,write_monographs_jsonl/read_monographs_jsonl): additive — serializelinesalongside the existingtext/headingfields per section. Existing consumers reading onlytext(e.g.segment/atc.py'sextract_atc_codes, which regexes overSectionSpan.text) need no change, per invariant 1.ingestion/data/processed/monographs.jsonl: schema grows a new optional-shaped field (sections[key].lines). Noschema_versionfield currently exists in the serialized dict (checkedio.pydirectly) — worth adding as part of this change, both for this migration and becausedocs/architecture.mdalready assumes "collection aliasing allows re-ingesting with a changed chunking strategy," which implies the ingestion output itself should be able to declare which schema shape it is.- Existing 110 tests: unaffected if invariant 1 holds — no assertion in
the current suite inspects
lines(it doesn't exist yet), andtext's value/semantics are unchanged. - New tests required (this ADR specifies them; implementation and the
actual test code are not part of this ADR):
- Regression test reproducing the ZOLPIDEM failure shape: a synthetic
span stream — last monograph's title and real sections, followed by
spans whose
printed_pageis out ofin_monograph_rangebut whose text matches avocab.pysection label — asserting the monograph closes with its real sections intact and the out-of-range spurious match is ignored, not accepted. SectionSpan.linesfixture test: using the real MORPHIN SULFAT boilerplate-fix fixture already intests/test_segment_assembler.py, assertlinespreserves the correctbold/physical_page/y0per retained line (and that stripped boilerplate lines are absent fromlinestoo, not just fromtext).- Round-trip test:
write_monographs_jsonl→read_monographs_jsonlpreserveslinesexactly (dataclass equality per line). - Whole-corpus invariant-1 check: for a real
cli runoutput, asserttext == "\n".join(l.text for l in lines).strip()holds for every section of every monograph, not a sample.
- Regression test reproducing the ZOLPIDEM failure shape: a synthetic
span stream — last monograph's title and real sections, followed by
spans whose
Relationship to ADR 0004
ADR 0004's chunk-unit decision ((drug_id, section_key)) is not
discarded — a section is still the natural parent grouping (matches how a
clinician thinks, matches Monograph.sections). What changes: ADR 0004
described a section as directly the chunk when under the 800-token
ceiling, with sentence-window splitting as the fallback for oversized
sections. Per the review above, splitting must instead first attempt to
break at real structural boundaries available in SectionSpan.lines (a
bold, short, isolated line — the same "subheading" shape already visually
confirmed for route-of-administration/population sub-headers — or an
explicit population/organ-function marker), with the sentence-window method
demoted to a fallback for the remaining prose that has no such marker. The
exact splitting algorithm (how a "subheading-shaped line" is defined
precisely, in code) is a chunk/-side implementation detail enabled by
this contract, not decided by it.
Not yet resolved (explicitly out of scope for this ADR)
- Table/formula content blocks. A separate whole-monograph-range survey
(pdfplumber
find_tables()+ PyMuPDF math-symbol scan, physical pages 98-1494 excluding blank page 99 — the exact setdetector. in_monograph_rangeaccepts, not an assumed offset) is in progress at the time of writing, per explicit user instruction to measure before deciding a table/formula chunk-unit strategy. This ADR'sBodyLinecontract covers text content only; a table/formula region should not currently be flattened intoBodyLines (doing so would repeat exactly the "destroys row/column meaning" mistake outlier item 7 already documents) — but the preciseContentBlock/table-row/formula-unit shape is deferred to a follow-up revision of this ADR once the survey reports real numbers (how many monographs/sections affected, page-break continuation frequency, multi-tier headers, merged cells, footnotes). - Paragraph-boundary detection (grouping consecutive
BodyLines into a flowing paragraph vs. a new one) is left tochunk/, using the same kind of y-gap heuristicsegment/merge.pyalready validates for multi-line title wraps (_MAX_LINE_GAP_PT) —BodyLine.y0is sufficient raw signal forchunk/to compute this itself;segment/does not need to pre-compute paragraph grouping. - The actual
chunk/splitting implementation (subheading detector, population-marker regex, sentence-window fallback) is not part of this ADR — this ADR defines the data contract that implementation will consume.