Files
duocthu/coordination/review-chunking-joint-2026-08-04.md
T

8.2 KiB
Raw Blame History

Joint chunking review — Codex + Claude Code — 2026-08-04

Decision

Do not embed the canonical corpus yet. Two defects affect the text that would be embedded: dose-bearing continuation chunks can lose their governing label, and two confirmed table descriptors contain quarantined ADR cell values misidentified as column headers.

The current artifact is structurally deterministic and lossless, but citation provenance and attachment propagation are not yet sufficient for user-facing RAG.

Review method

  • Codex inspected the implementation, canonical artifact and rendered table crops, and mapped every prose chunk back to SectionPart.physical_page.
  • An independent peer review checked chunk/schema/load invariants read-only.
  • Claude Code independently read the review scope, regenerated the corpus in memory, aligned all continuation chunks, ran the test suites, and inspected the two ARSENIC TRIOXYD crops. Claude made no repository edits.
  • No Bedrock call, embedding run, IAM change, commit or push was performed.

Blocking findings

S1 — Dose continuation can omit its governing label — blocks embedding

Location: ingestion/ingestion/chunk/chunker.py:109-120.

The overlap window walks backward using only the overlap token budget. When the next atom would exceed that budget, a short :-terminated population, route or indication label can remain only in the previous chunk while the next chunk begins with its dose.

Claude aligned all 2,941 continuation chunks to source text:

  • 289 begin exactly after a stranded :-terminated label and omit that label;
  • 195 contain a dose/strength figure in the first 200 characters;
  • 37 strand a population label and begin with a dose.

Confirmed examples include:

  • zidovudin__lieu_luong_va_cach_dung__2: omits Trẻ đẻ thiếu tháng: and begins with Uống liều ban đầu 2 mg/kg cách 12 giờ một lần.;
  • pancuronium__lieu_luong_va_cach_dung__1: omits Trẻ em dưới 1 tháng tuổi: and begins with the neonatal induction dose;
  • amikacin__lieu_luong_va_cach_dung__1: omits Trẻ sơ sinh và trẻ đẻ non:;
  • morphin_sulfat__lieu_luong_va_cach_dung__4: omits the indication/form label governing 10 - 30 mg, uống 4 giờ một lần..

The earlier count of 14 chunks ending in : examined the opposite seam. Those 14 are benign final prose parts introducing quarantined tables; it does not cover the 289 continuation starts above.

S2 — Quarantined table cells leak into descriptor text — blocks embedding

Locations: ingestion/ingestion/chunk/chunker.py:41-48, :147-149, :176-179; blind gate at ingestion/ingestion/validation/readiness.py:201-206.

_is_label_row treats any short digit-free first row as a header. In rendered ARSENIC TRIOXYD continuation tables p209_t0 and p209_t1, the first visible rows are body data, but the descriptors ship them as Cột::

  • Ngoại tâm thu thất | Thường gặp | Không rõ tần suất;
  • Tăng bilirubin máu | Thường gặp | Thường gặp.

There are 71 descriptors with a non-empty header_row; two violations are visually confirmed. The remaining 67 first-part/header-bearing cases were not all visually audited. The readiness probe searches only one contiguous raw prefix, while descriptor construction inserts |, so these leaks pass the current gate by construction.

All descriptors currently force VERIFY_PDF, so the bad text is not copied into the answer string. It still contaminates embedding/retrieval and violates the quarantine invariant.

Must fix before user-facing RAG

S3 — Citation range is monograph-wide, not chunk-exact

Locations: ingestion/ingestion/chunk/chunker.py:193-216, descriptor path :237-256.

All 14,915 prose chunks were uniquely mapped back to section parts:

  • only 251 declared ranges equal their actual supporting pages;
  • 14,664 inherit 16 unrelated monograph pages;
  • all 151 descriptors use the monograph range instead of the attachment page;
  • 142/151 descriptors state a page in their text that differs from the range start exposed as the primary citation page.

Example: the ACETAZOLAMID descriptor says printed page 110 but carries printed_page_range=[109,111]. This does not change vectors, but it blocks honest citation and PDF verification UX.

S4 — ai-service drops attachment provenance

Location: apps/ai-service/adapters/qdrant.py:37-50.

The adapter ignores payload attachments and instead constructs a fallback source reference from the section heading page plus broad ranges. Consequently block_id, bbox and source_crop are absent, and the physical page is wrong for 65/151 descriptors. The response can request PDF verification without linking to the quarantined crop/region.

S5 — Schema v3/load path does not fail closed

Locations: ingestion/ingestion/chunk/models.py:47-64, ingestion/ingestion/chunk/chunker.py:185-203, and ingestion/ingestion/load/models.py:30-42,161-177.

printed_page_range defaults to []; direct chunk_all() can omit the printed map; and loader validation neither requires schema v3 nor a non-empty printed range. Empty source_page_range and other empty lists also pass. The current canonical artifact is complete, but a future direct regeneration/load can spend on embeddings and then make every answer abstain for missing provenance.

Non-blocking or latent findings

  • _atoms can drop a comma for synthetic empty fragments such as ,, after a long split (chunker.py:70-79). It does not fire in the current 11,974 non-empty sections; the regression assertion strips commas and cannot catch it.
  • Corpus SHA is line-ending-dependent: identical JSONL data hashes differently with Windows CRLF versus Linux LF, which can falsely reject a CI/container load.
  • _pack can emit a label-only part in a synthetic single-label buffer. No such occurrence exists in the current artifact; this is separate from S1.
  • Adding printed_page_map before measure breaks old positional third-argument callers. No in-repo caller is affected.
  • ADR 0004/0006 and docs/v1-delivery-plan.md contain stale schema, table-count, token-estimator and page-tracking claims.
  • The earlier phrase “Ruff clean” applied to the selected changed paths. Claude confirmed that an unconfigured whole-directory ruff check . is not clean (397 findings), so it must not be represented as a repository-wide gate.

Areas that passed review

  • Canonical SHA confirmed: e474c83790b450d3262f532e81abf6526a485e3a98e376413247da23f4619c38.
  • 15,066 records: 14,915 prose + 151 descriptors; all schema v3.
  • Zero duplicate chunk IDs and zero UUID5 point-ID collisions.
  • part_index/part_count are consistent; descriptors are (0,1).
  • Full in-memory regeneration is byte-identical on the same CRLF platform.
  • Strong independent reassembly check found zero source-substring failures, coverage gaps, reordering, or unintended duplication across all 11,974 non-empty sections.
  • Zero chunks exceed 800 estimated tokens; maximum is exactly 800.
  • No confirmed quarantined numeric cell content leaked into prose chunks.
  • Printed folio extraction is derived from visible page headers and fails to None on ambiguity rather than guessing.
  • Current answer construction does not return quarantined descriptor text to the user; it forces VERIFY_PDF.
  • Ingestion tests: 258 passed. Claude's isolated ai-service run had 19 passed and 3 live-integration skips; the earlier configured local-service run had all 22 passing.
  1. Make overlap label-aware at both sides of every seam and add corpus-level tests for population + dose adjacency (S1).
  2. Stop inferring headers for continuation tables without reliable logical-table linkage; repair the two confirmed descriptors and strengthen the leak gate (S2).
  3. Compute exact per-chunk printed/physical provenance from SectionParts and exact block provenance from attachments (S3).
  4. Preserve attachment block/page/bbox/crop through Qdrant and citation assembly (S4).
  5. Require schema v3 plus non-empty, valid page ranges at model, chunk and loader boundaries (S5).
  6. Regenerate the canonical artifact, rerun readiness/tests and repeat this review before embedding any corpus records.