Files
duocthu/docs-legacy/adr/0005-segment-output-contract-for-chunking.md
T
VuQuangBao a85b0ccac8 Fix the F3 out-of-scope gate, close out the V1 feature audit, and clean up project docs
Also drop .github/ (GitHub-specific CI/CD workflows and ArgoCD
operational scripts) from this mirror -- Gitea auto-picked up
.github/workflows/*.yml as Actions and queued a run against secrets
that don't exist here. Not meaningful outside the GitHub-hosted repo
anyway.
2026-08-25 12:05:00 +07:00

216 lines
12 KiB
Markdown

# 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:
1. **A section is not a single semantic unit.** `liều lượng và cách dùng`
and `tương tác thuốc` routinely 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 a `liều
lượng và cách dùng` section — this is common, not an edge case.
2. **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.
3. **Tables inside the monograph range are not addressed at all.** ADR 0004
implicitly assumed monograph-range sections are prose. `docs/pdf-parsing-
outlier-catalog.md` item 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).
4. **Provenance is section-level, not chunk-level**, because per-line
`physical_page`/`y0` (which `Span` already carries — see
`extract/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's `source_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`)
```python
@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). 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
1. `text == "\n".join(l.text for l in lines).strip()` for every
`SectionSpan`, for the lifetime of this contract — `lines` is 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, not `text` alone.
2. `lines` is in reading order, matching the order `text`'s lines already
implicitly have.
3. Every `BodyLine.physical_page` satisfies `detector.in_monograph_range`
for a `Span` on that page — i.e., **no line in any `SectionSpan.lines`
may 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.
4. 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 — serialize
`lines` alongside the existing `text`/`heading` fields per section.
Existing consumers reading only `text` (e.g. `segment/atc.py`'s
`extract_atc_codes`, which regexes over `SectionSpan.text`) need no
change, per invariant 1.
- **`ingestion/data/processed/monographs.jsonl`**: schema grows a new
optional-shaped field (`sections[key].lines`). No `schema_version` field
currently exists in the serialized dict (checked `io.py` directly) —
worth adding as part of this change, both for this migration and because
`docs/architecture.md` already 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), and `text`'s
value/semantics are unchanged.
- **New tests required** (this ADR specifies them; implementation and the
actual test code are not part of this ADR):
1. Regression test reproducing the ZOLPIDEM failure shape: a synthetic
span stream — last monograph's title and real sections, followed by
spans whose `printed_page` is out of `in_monograph_range` but whose
text matches a `vocab.py` section label — asserting the monograph
closes with its real sections intact and the out-of-range spurious
match is ignored, not accepted.
2. `SectionSpan.lines` fixture test: using the real MORPHIN SULFAT
boilerplate-fix fixture already in `tests/test_segment_assembler.py`,
assert `lines` preserves the correct `bold`/`physical_page`/`y0` per
retained line (and that stripped boilerplate lines are absent from
`lines` too, not just from `text`).
3. Round-trip test: `write_monographs_jsonl``read_monographs_jsonl`
preserves `lines` exactly (dataclass equality per line).
4. Whole-corpus invariant-1 check: for a real `cli run` output, assert
`text == "\n".join(l.text for l in lines).strip()` holds for every
section of every monograph, not a sample.
## 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 set `detector.
in_monograph_range` accepts, 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's `BodyLine`
contract covers **text content only**; a table/formula region should
*not* currently be flattened into `BodyLine`s (doing so would repeat
exactly the "destroys row/column meaning" mistake outlier item 7 already
documents) — but the precise `ContentBlock`/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 `BodyLine`s into a
flowing paragraph vs. a new one) is left to `chunk/`, using the same
kind of y-gap heuristic `segment/merge.py` already validates for
multi-line title wraps (`_MAX_LINE_GAP_PT`) — `BodyLine.y0` is sufficient
raw signal for `chunk/` 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.