docs(tasks): P7-3 pdf ingest wiring task spec #39
@@ -70,9 +70,10 @@ P0~P5 는 직렬. P6~P9 는 P5 이후 병렬 가능.
|
||||
- [p6-2 ocr-adapter](p6/p6-2-ocr-adapter.md)
|
||||
- [p6-3 caption-adapter](p6/p6-3-caption-adapter.md)
|
||||
- [p6-4 image-ingest-wiring](p6/p6-4-image-ingest-wiring.md)
|
||||
- P7 — [p7/](p7/) — 2 components
|
||||
- P7 — [p7/](p7/) — 3 components
|
||||
- [p7-1 pdf-text-extractor](p7/p7-1-pdf-text-extractor.md)
|
||||
- [p7-2 pdf-page-chunker](p7/p7-2-pdf-page-chunker.md)
|
||||
- [p7-3 pdf-ingest-wiring](p7/p7-3-pdf-ingest-wiring.md)
|
||||
- P8 — [p8/](p8/) — 2 components
|
||||
- [p8-1 whisper-adapter](p8/p8-1-whisper-adapter.md)
|
||||
- [p8-2 segment-chunker](p8/p8-2-segment-chunker.md)
|
||||
|
||||
215
tasks/p7/p7-3-pdf-ingest-wiring.md
Normal file
@@ -0,0 +1,215 @@
|
||||
---
|
||||
phase: P7
|
||||
component: kebab-app (PDF ingest dispatch + chunker selection)
|
||||
task_id: p7-3
|
||||
title: "Wire PdfTextExtractor + PdfPageV1Chunker into kebab-app::ingest end-to-end"
|
||||
status: planned
|
||||
depends_on: [p7-1, p7-2, p1-6, p3-5, p6-4]
|
||||
unblocks: []
|
||||
contract_source: ../../docs/superpowers/specs/2026-04-27-kebab-final-form-design.md
|
||||
contract_sections: [§3.4 SourceSpan::Page, §3.5 Chunk, §6.1 ingest pipeline, §7.2 Extractor/Chunker traits, §9.2 PDF text extraction]
|
||||
---
|
||||
|
||||
# p7-3 — PDF ingest wiring (kebab-app)
|
||||
|
||||
## Goal
|
||||
|
||||
Make `kebab ingest` end-to-end functional for PDF assets. P7-1 (`PdfTextExtractor`) and P7-2 (`PdfPageV1Chunker`) each ship a tested library; this task connects the wires from `kebab-source-fs` (which already classifies `.pdf` as `MediaType::Pdf`) through `kebab-app::ingest_with_config` to `kebab-store-sqlite` + `kebab-store-vector`, so a user running `kebab ingest` against a workspace containing PDF papers / books / reports sees those assets indexed and searchable, with page-level citations preserved.
|
||||
|
||||
## Why now / why this size
|
||||
|
||||
P7-1 / P7-2 deliver value only after `kebab-app::ingest` learns to dispatch on `MediaType::Pdf`. The wiring is small — one new dispatch arm in `ingest_one_asset`, one new private helper `ingest_one_pdf_asset`, and a per-medium chunker selection step — but it is materially user-facing: without it, the entire P7 phase is invisible from the CLI. P6-4 (image wiring) established the pattern; P7-3 follows it identically with two structural differences:
|
||||
|
||||
1. PDF chunking uses a **separate** chunker (`pdf-page-v1`) rather than an in-place branch inside `md-heading-v1`. The chunker selection happens at dispatch time keyed on `MediaType`.
|
||||
2. PDF documents commonly produce **many** chunks per asset (one per page, sometimes more for long pages); the embedder loop must scale to hundreds of chunks per doc without breaking the per-asset transaction.
|
||||
|
||||
Pulling this into its own task keeps P7-1 / P7-2 specs frozen as written while letting integration evolve under its own contract.
|
||||
|
||||
## Allowed dependencies
|
||||
|
||||
`kebab-app` 의 현재 Cargo.toml + `kebab-parse-pdf` 한 줄 추가.
|
||||
|
||||
- `kebab-core`
|
||||
- `kebab-config`
|
||||
- `kebab-source-fs`
|
||||
- `kebab-parse-md`, `kebab-parse-types`
|
||||
- `kebab-normalize`
|
||||
- `kebab-chunk` (uses both `MdHeadingV1Chunker` AND `PdfPageV1Chunker` — chunker selection at dispatch time)
|
||||
- `kebab-store-sqlite`, `kebab-store-vector`
|
||||
- `kebab-search`
|
||||
- `kebab-embed`, `kebab-embed-local`
|
||||
- `kebab-llm`, `kebab-llm-local`
|
||||
- `kebab-rag`
|
||||
- `kebab-parse-image`
|
||||
- **`kebab-parse-pdf` (NEW — added by this task)**
|
||||
- `anyhow`, `serde_json`, `tracing`
|
||||
|
||||
## Forbidden dependencies
|
||||
|
||||
- `kebab-tui`, `kebab-desktop` (P9 미시작 — UI crate 가 ingest 호출하면 layering 위반).
|
||||
- `kebab-eval` (cycle 위험 — eval 이 ingest 를 호출).
|
||||
- 본 task 안에서 PDF parsing / chunking 로직을 재구현 금지. `kebab-parse-pdf` + `kebab-chunk::PdfPageV1Chunker` 의 thin dispatch + glue 만 허용.
|
||||
|
||||
## Inputs
|
||||
|
||||
| input | type | source |
|
||||
|-------|------|--------|
|
||||
| workspace assets | `RawAsset` stream | `kebab-source-fs::SourceConnector::scan` (already classifies `MediaType::Pdf`) |
|
||||
| PDF bytes | `&[u8]` | filesystem read in `kebab-app` |
|
||||
| `Config` | `kebab_config::Config` | CLI `--config` flag → `Config::load` |
|
||||
| `ChunkPolicy` | `kebab_core::ChunkPolicy` | derived from `config.chunking` (existing) |
|
||||
|
||||
## Outputs
|
||||
|
||||
| output | type | downstream |
|
||||
|--------|------|------------|
|
||||
| `CanonicalDocument` per PDF | written via `DocumentStore::put_document` | `kebab-store-sqlite` |
|
||||
| `Vec<Chunk>` per PDF (≥1 chunk per non-empty page) | `kebab-store-sqlite::put_chunks` + `kebab-store-vector::upsert` | `kebab-store-sqlite` + `kebab-store-vector` |
|
||||
| Updated `IngestReport` counters | `scanned / new / updated / skipped / errors` | wire output (`ingest_report.v1`) |
|
||||
|
||||
## Public surface
|
||||
|
||||
No new public types. The wiring exists inside `kebab-app::ingest_with_config` (and its private helpers). One new private function:
|
||||
|
||||
```rust
|
||||
// crates/kebab-app/src/lib.rs (additions only — sketch)
|
||||
|
||||
/// P7-3: process one `MediaType::Pdf` asset end-to-end.
|
||||
fn ingest_one_pdf_asset(
|
||||
app: &App,
|
||||
asset: &RawAsset,
|
||||
chunk_policy: &ChunkPolicy,
|
||||
embedder: Option<&Arc<dyn Embedder + Send + Sync>>,
|
||||
vector_store: Option<&Arc<kebab_store_vector::LanceVectorStore>>,
|
||||
existing_doc_ids: &std::collections::HashSet<String>,
|
||||
) -> anyhow::Result<kebab_core::IngestItem> { ... }
|
||||
```
|
||||
|
||||
`ingest_one_asset` gets a new `match` arm:
|
||||
|
||||
```rust
|
||||
match &asset.media_type {
|
||||
MediaType::Markdown => { /* existing fall-through */ }
|
||||
MediaType::Image(_) => return ingest_one_image_asset(...),
|
||||
MediaType::Pdf => return ingest_one_pdf_asset(...), // NEW
|
||||
_ => return Ok(IngestItem { kind: Skipped, ... }),
|
||||
}
|
||||
```
|
||||
|
||||
## Behavior contract
|
||||
|
||||
### Ingest dispatch (kebab-app)
|
||||
|
||||
- For each `RawAsset`:
|
||||
- `MediaType::Markdown` → existing markdown extractor path (unchanged).
|
||||
- `MediaType::Image(_)` → P6-4 image branch (unchanged).
|
||||
- `MediaType::Pdf` → new PDF branch (this task).
|
||||
- `MediaType::Audio(_) | MediaType::Other(_)` → `skipped += 1` (existing behaviour).
|
||||
- **Operational jump**: before this task, `MediaType::Pdf` fell into the `_` arm and was counted as `Skipped`. After merge, every PDF asset shifts from `skipped` to `scanned` / `new` / `updated`. A workspace with N PDF files reports `skipped` decreasing by N and `scanned` (and `new` on the first ingest after merge) increasing by N — flag this in the implementation PR description so eval / smoke / runtime users can interpret the one-time discontinuity.
|
||||
- PDF branch (`ingest_one_pdf_asset`):
|
||||
1. Read bytes via `std::fs::read` (consistent with markdown / image branches).
|
||||
2. Build `kebab_core::ExtractContext { asset, workspace_root, config: &ExtractConfig::default() }`.
|
||||
3. Call `PdfTextExtractor::new().extract(&ctx, &bytes)`. Failure (`Err(_)`) → `IngestItemKind::Error` with the formatted error in `IngestItem.error`. Continue to next asset (do not abort the whole ingest).
|
||||
- Encrypted PDFs hit this branch (P7-1 returns `Err` with the `qpdf --decrypt` hint preserved verbatim — the operator sees the actionable message in the `kebab ingest` output).
|
||||
- Corrupt / non-PDF bytes likewise.
|
||||
4. The returned `CanonicalDocument` may carry per-page `Provenance::Warning` events (P7-1 emits one for each empty / extract-failed page, marked "scanned candidate"). Pass these through unchanged — the chunker correctly emits 0 chunks for empty pages, so the asset is still indexed but those pages are not searchable until a future scanned-PDF OCR fallback lands (out of scope here).
|
||||
5. Pass the `CanonicalDocument` to **`PdfPageV1Chunker`** (NOT `MdHeadingV1Chunker` — chunker selection is keyed on `MediaType::Pdf`). The chunker validates that every block is `Block::Paragraph` with `SourceSpan::Page`; if validation fails (which would mean P7-1's contract drifted), the chunker's error propagates up as `IngestItemKind::Error`.
|
||||
6. Persist `CanonicalDocument` + `Vec<Chunk>` via the same `DocumentStore::put_document` + `put_chunks` calls the markdown branch uses.
|
||||
7. Embed each chunk if `embedder.is_some()`. Each PDF chunk gets one vector — the embedder loop processes them in batches of `config.indexing.max_parallel_embeddings` like markdown chunks (no PDF-specific batching).
|
||||
|
||||
### Chunker selection
|
||||
|
||||
- Per-medium chunker selection is the new architectural piece. Today the markdown branch hard-codes `MdHeadingV1Chunker`; the PDF branch hard-codes `PdfPageV1Chunker`. There is no runtime config switch — the medium → chunker mapping is compiled in.
|
||||
- A future task (P+ "chunker registry") may make this configurable, at which point the mapping moves to `Config::chunking.chunker_for_media`. P7-3 deliberately does not introduce that config slot — premature config surface.
|
||||
- `config.chunking.chunker_version` is a fingerprint, not a dispatcher. Markdown sets it to `"md-heading-v1"`, PDF would set it to `"pdf-page-v1"` — but in the current `Config` schema the field is single-valued and serves the markdown path only. **Deviation logged in HOTFIXES**: PDF ingest ignores `config.chunking.chunker_version`, hard-codes `pdf-page-v1`. A future P+ task either splits the config field per medium or builds the chunker registry above.
|
||||
|
||||
### Determinism stress
|
||||
|
||||
- The existing markdown path's `kb-normalize::build_canonical_document` and the image path's `ingest_one_image_asset` share one `OffsetDateTime::now_utc()` reading per Provenance event group. P7-1's extractor already shares one `now` reading across its Discovered + Parsed + per-page Warning events. The PDF branch must not insert a second `now()` between extract and chunk — chunking is a pure function of the `CanonicalDocument`, so this constraint is structural, not stylistic.
|
||||
- Re-ingest of the same PDF bytes produces identical `doc_id`, identical `block_id`s (per-page deterministic via `id_for_block(doc_id, "paragraph", &[], page-1, span)`), and identical `chunk_id`s (P7-2's per-chunk policy_hash variant `#c{char_start}` makes `chunk_id` deterministic-and-collision-free across the document).
|
||||
|
||||
### Failure semantics summary
|
||||
|
||||
| failure | counter | doc stored? | provenance |
|
||||
|---|---|---|---|
|
||||
| `PdfTextExtractor::extract` Err (corrupt header / not a PDF) | `errors+=1` | no | n/a (no doc emitted) |
|
||||
| `PdfTextExtractor::extract` Err (encrypted PDF) | `errors+=1` | no | n/a — error message includes `qpdf --decrypt` hint |
|
||||
| `PdfPageV1Chunker::chunk` Err (validates non-Page span / non-Paragraph block) | `errors+=1` | no | n/a — defensive validation: fires on P7-1 contract drift OR future routing bug (e.g. a chunker registry mis-routes a markdown doc here) |
|
||||
| Per-page text extraction Err (panic absorbed by `catch_unwind`) | unchanged | yes | `Provenance::Warning` (page N "scanned candidate") — emitted by P7-1, propagated through |
|
||||
| Empty page (no `/Contents` stream) | unchanged | yes | `Provenance::Warning` (page N "empty (scanned candidate)") — emitted by P7-1 |
|
||||
| `Embedder::embed(...)` Err (any chunk) | `errors+=1` | yes (doc + chunk rows already written before embed call — see below) | n/a |
|
||||
|
|
||||
|
||||
The embedding call sits *after* `put_document` / `put_blocks` / `put_chunks` in `kebab-app::ingest_one_asset` (markdown path, lines 615+), so a failed embed leaves doc + chunk rows on disk while no vector exists. This is consistent with the markdown path and accepted as v1 behaviour — re-running `kebab ingest` re-attempts the embed for any chunk whose `embedding_id` is missing from the vector store. Whole-asset rollback on embed-fail is a P+ task (atomic ingest transaction).
|
||||
|
claude-reviewer-01
commented
(칭찬) Dispatch 절에 "이전엔 PDF 가 Skipped 였음 → 머지 후 skipped→scanned 이동" 의 운영 jump 한 줄을 정확히 수치화 ("N PDF files reports (칭찬) Dispatch 절에 "이전엔 PDF 가 Skipped 였음 → 머지 후 skipped→scanned 이동" 의 운영 jump 한 줄을 정확히 수치화 ("N PDF files reports `skipped` decreasing by N") 한 게 좋습니다. P5 eval / smoke / 사용자가 머지 직후 ingest 통계 변화를 보고 "왜 갑자기 숫자가 바뀌지" 의 답을 spec 만 봐도 즉시 찾을 수 있는 상태.
|
||||
|
||||
## Storage / wire effects
|
||||
|
||||
- `kebab-store-sqlite::documents` table gains rows whose `parser_version = "pdf-text-v1"`.
|
||||
- `kebab-store-sqlite::blocks` table gains rows of `block_kind = "paragraph"` whose `source_span` is `Page { page, char_start, char_end }`. This is the first time the workspace stores blocks with `SourceSpan::Page`; any downstream reader that did not handle this variant is exposed.
|
||||
- `kebab-store-sqlite::chunks` table gains rows whose `chunker_version = "pdf-page-v1"` AND whose `source_spans[0]` is `SourceSpan::Page`. Same exposure note for downstream readers.
|
||||
- `kebab-store-vector::chunk_embeddings_<model>_<dim>` gains one vector per PDF chunk.
|
||||
- `IngestReport` (wire `ingest_report.v1`) counters update naturally: `scanned` includes PDFs, `new` / `updated` track PDF docs, `errors` counts decode / encryption / corrupt failures, `skipped` continues to count audio / unknown formats.
|
||||
- No new wire schemas or `kebab-core` types.
|
||||
|
||||
### Citation surface
|
||||
|
||||
- `Citation` (search hits / RAG answers) already carries `SourceSpan::Page`. The CLI / wire layer must render it — current `kebab search` JSON output passes `source_span` through verbatim; no change needed in this task. UI rendering of "page 12" labels for PDF citations is the responsibility of P9 (TUI / desktop) or whatever consumer is reading the wire.
|
||||
|
||||
|
claude-reviewer-01
commented
(칭찬) (칭찬) `config.chunking.chunker_version` deviation 을 spec 에 미리 못 박은 게 정직합니다. 본 task 가 머지된 직후 사용자가 `config.toml` 의 `chunker_version = "md-heading-v1"` 을 보고 "PDF 도 같이 바뀌나?" 라고 묻을 때 정확한 답 ("PDF 는 무시, hard-coded `pdf-page-v1`") 가 spec 에 박혀 있고, HOTFIXES entry + 향후 chunker registry task 까지 명확히 path 가 그려진 상태. 이런 "문서화 가능한 작은 거짓말" 을 task 단위에서 받아들이는 자세 + path 명시가 워크스페이스의 실용주의 ("Ship working software, document deviations honestly") 와 정렬.
|
||||
## Test plan
|
||||
|
||||
| kind | description | fixture / data |
|
||||
|------|-------------|----------------|
|
||||
| integration | TempDir KB + 1 small text PDF (3 pages) → `kebab ingest` produces 1 doc + 3 chunks; each chunk's `source_spans[0]` is `Page { page: i, .. }`; chunks stored + embedded | `kebab-app/tests/pdf_pipeline.rs` (new), in-memory PDF via `lopdf` builder (mirrors `kebab-parse-pdf::tests::common`) |
|
||||
|
claude-reviewer-01
commented
(칭찬) Determinism stress 절이 P6-4 와 동일한 invariant ( (칭찬) Determinism stress 절이 P6-4 와 동일한 invariant (`now()` 추가 호출 금지, extract → chunk 사이) 를 명시적으로 못 박은 게 좋습니다. "chunking 은 pure function of CanonicalDocument 이므로 structural 한 제약" 라는 표현이 "왜 now() 호출이 추가되면 안 되는지" 의 근본 원인 (Provenance event 의 `at` 타임스탬프가 group 단위로 공유되는 invariant) 을 정확히 짚습니다. 미래 reader 가 "이 invariant 가 왜 있더라" 를 spec 만 읽고도 알 수 있는 상태.
|
||||
| integration | Re-ingest same PDF (identical bytes) → identical `doc_id` and identical `chunk_id` set (P1 idempotency contract) | inline |
|
||||
| integration | Edit a PDF (replace bytes — different blake3 → different `asset_id` → different `doc_id`) and re-ingest → `new+=1` for the new `doc_id`; old `doc_id` row remains untouched (orphan handling is a P+ task) | inline |
|
||||
| integration | Encrypted PDF → asset NOT stored; `errors+=1`; `IngestItem.error` mentions `qpdf` / `decrypt` | inline (lopdf builder + fake `/Encrypt` trailer) |
|
||||
| integration | Corrupt header PDF → asset NOT stored; `errors+=1`; error message mentions PDF parse failure | inline |
|
||||
| integration | Mixed page PDF (page 1 text, page 2 empty / scanned, page 3 text) → asset stored; 2 chunks (pages 1 + 3); `doc.provenance.events` contains exactly 1 `Warning` for page 2 marked "scanned candidate" | inline |
|
||||
| integration | `kebab inspect doc <pdf_doc_id>` returns the PDF `CanonicalDocument` with per-page `Block::Paragraph` and `SourceSpan::Page` intact | inline |
|
||||
| integration | Hybrid search across mixed corpus (1 markdown + 1 PDF) returns the PDF chunk for a query whose terms appear only in the PDF body | inline (real `multilingual-e5-small` embedding) |
|
||||
| integration | `IngestReport` invariant `scanned == new + updated + skipped + errors` holds when ingesting a mixed corpus including a corrupt PDF | inline |
|
||||
|
claude-reviewer-01
commented
(nit) How to apply: 행의 비고를 "P7-1 contract drift OR future routing bug — defensive validation either way" 로 보강. (nit) `PdfPageV1Chunker::chunk Err` 행이 "would only fire on P7-1 contract drift" 만 표현. 사실은 *defensive validation* 이라 미래에 누군가 잘못된 doc 을 넘겨도 (예: P+ chunker registry task 의 routing bug) 즉시 잡힙니다. 이 측면도 한 줄 명시하면 spec 가 frozen 된 후에도 chunker 의 가드 의도가 정확히 전달됩니다.
How to apply: 행의 비고를 "P7-1 contract drift OR future routing bug — defensive validation either way" 로 보강.
claude-reviewer-01
commented
(칭찬) (칭찬) `PdfPageV1Chunker::chunk Err` 비고를 "P7-1 contract drift OR future routing bug — defensive validation either way" 로 정확화. 미래 chunker registry task 가 routing bug 를 만들 때 "왜 chunker 가 거부 했지?" 를 spec 에서 즉시 찾을 수 있게 됐습니다.
|
||||
| integration | Long PDF (50 pages × ~1.5 KB body each = ~75 KB) produces ≥50 chunks (≥1 per page); embedding loop completes; storage round-trips | inline |
|
||||
| smoke | Update `docs/SMOKE.md` so the runbook ingests at least one PDF fixture and verifies search-by-page-text works + `inspect doc` shows `SourceSpan::Page` | docs change |
|
||||
|
||||
|
claude-reviewer-01
commented
(nit / 모호성) Failure semantics 표의 마지막 행 "Embedding call fails for a chunk → bubbles up via existing markdown path's error handling / partial — depends on existing behaviour" 가 implementation 시점에 결정을 미루는 모호한 표현입니다. md path 의 현재 동작을 한 줄로 명시하거나, "P7-3 implementation 이 결정 + HOTFIXES 기록" 식으로 명시. Why: spec 은 frozen contract 라 "depends on existing behaviour" 같은 회피 표현은 미래 reader 가 디버깅 시 무엇을 기대해야 할지 결정 못 하게 만듭니다. md path 가 어떻게 동작하는지 (PR diff 본문에는 fail-fast 인지 partial 인지) 한 줄 명시 권장. How to apply: (nit / 모호성) Failure semantics 표의 마지막 행 "Embedding call fails for a chunk → bubbles up via existing markdown path's error handling / partial — depends on existing behaviour" 가 implementation 시점에 결정을 미루는 모호한 표현입니다. md path 의 현재 동작을 한 줄로 명시하거나, "P7-3 implementation 이 결정 + HOTFIXES 기록" 식으로 명시.
Why: spec 은 frozen contract 라 "depends on existing behaviour" 같은 회피 표현은 미래 reader 가 디버깅 시 무엇을 기대해야 할지 결정 못 하게 만듭니다. md path 가 어떻게 동작하는지 (PR diff 본문에는 fail-fast 인지 partial 인지) 한 줄 명시 권장.
How to apply: `crates/kebab-app/src/lib.rs` 의 markdown path 를 한 번 읽고 (`embed_chunks` 또는 유사 호출 부근) 다음 형식으로 채우기 — 예: "chunk 단위 embedding fail → 해당 chunk 의 vector 만 누락, doc 는 stored, IngestItemKind::Error 가 아닌 Updated 또는 New 로 분류; `IngestReport.errors` 는 unchanged. 정상 동작."
|
||||
The opt-in real-Ollama integration tests stay in `kebab-llm-local` / `kebab-parse-image`. P7-3 adds zero LM dependency — PDF text extraction is local-only, so there is no equivalent hermetic-vs-real split to manage.
|
||||
|
||||
## Definition of Done
|
||||
|
claude-reviewer-01
commented
(칭찬) Embedding fail 행을 " (칭찬) Embedding fail 행을 "`Embedder::embed Err → errors+=1, doc + chunks rows 는 이미 저장됨, 재실행 시 재시도" 로 정확화 + md path 코드 위치 (`lib.rs` 615+) 명시. "depends on existing behaviour" 같은 회피 표현이 사라지고 spec 가 frozen 후에도 미래 reader 가 "부분 저장 + 재실행 retry" 라는 v1 정책의 정확한 의도를 알 수 있는 상태. 추가로 "Whole-asset rollback on embed-fail 은 P+ task" 라는 향후 path 까지 표시한 게 정직.
|
||||
|
||||
### Spec PR (this PR — `spec/p7-3-pdf-ingest-wiring`)
|
||||
|
||||
|
claude-reviewer-01
commented
(칭찬) Storage / wire effects 절이 " (칭찬) Storage / wire effects 절이 "`SourceSpan::Page` 가 store 에 처음 저장된다" 는 사실을 명시한 게 중요합니다. P1-6 시점에 polymorphic source_span_kind/payload 컬럼이 만들어졌지만 `Page` variant 가 실제로 그 컬럼을 사용하는 건 P7-3 가 처음 — 즉 P1-6 의 polymorphic 약속이 실전에서 검증되는 첫 PR 이라는 의미입니다. "any downstream reader that did not handle this variant is exposed" 한 줄이 미래에 `inspect` / `search` JSON 출력 디버깅 시 정확한 출발점을 알려줍니다.
|
||||
- [ ] `tasks/p7/p7-3-pdf-ingest-wiring.md` 작성 + self-review (placeholder / 모순 / 모호성 / scope)
|
||||
- [ ] `tasks/INDEX.md` "P7 — 3 components" 반영
|
||||
- [ ] PR 본문에 design §3.4, §3.5, §6.1, §9.2 링크
|
||||
- [ ] HOTFIXES note 가 필요한 deviation (chunker selection, `config.chunking.chunker_version` PDF 무시) 본문에 명시
|
||||
|
||||
### Implementation PR (follow-up — `feat/p7-3-pdf-ingest-wiring`)
|
||||
|
||||
- [ ] `cargo check --workspace` passes
|
||||
- [ ] `cargo test --workspace --no-fail-fast -j 1` passes (all new integration tests green)
|
||||
- [ ] `cargo clippy --workspace --all-targets -- -D warnings` passes
|
||||
- [ ] `kebab ingest` against a TempDir KB containing 1 markdown + 1 image + 1 PDF produces `scanned 3 / new 3 / errors 0`
|
||||
- [ ] `kebab search --mode hybrid "<text from PDF page 7>"` returns a chunk whose `source_span` is `Page { page: 7, .. }`
|
||||
- [ ] `kebab inspect doc <pdf_doc_id>` shows per-page `Block::Paragraph` with `SourceSpan::Page`
|
||||
- [ ] HOTFIXES entry written for the `config.chunking.chunker_version` deviation (PDF ignores; hard-coded `pdf-page-v1`)
|
||||
|
claude-reviewer-01
commented
(coverage gap) Re-ingest 테스트는 "동일 byte" 케이스만 명시. md / image path 가 모두 검증하는 "수정된 PDF re-ingest → updated 카운터 증가" 케이스가 빠져있습니다. P1 idempotency contract 의 다른 axis ( Why: PDF doc_id 는 How to apply: Test plan 표 한 줄 추가: 또는 "동일 path 같은 asset_id 다른 parser_version" 같은 P+ 시나리오는 이번 task 에 포함시키지 않아도 OK — byte 수정 케이스만 추가. (coverage gap) Re-ingest 테스트는 "동일 byte" 케이스만 명시. md / image path 가 모두 검증하는 "수정된 PDF re-ingest → updated 카운터 증가" 케이스가 빠져있습니다. P1 idempotency contract 의 다른 axis (`updated == 1` for edited content) 가 PDF path 에서도 동작하는지 implementation 수준에서 한 번 잠가줘야 합니다.
Why: PDF doc_id 는 `(workspace_path, asset_id, parser_version)` 의 함수인데 asset_id 는 `id_for_asset(blake3_full_hex)` 로 byte-기반. byte 가 바뀌면 asset_id 도 바뀌고 따라서 doc_id 도 바뀝니다 → 새 row, `new+=1`. 만약 그렇지 않다면 (workspace_path 만 바뀌어도 asset_id 보존되는 path 가 있다면) updated 분기가 동작해야 함. 본 invariant 가 PDF 에서도 깨지지 않는지 시각화하는 게 중요.
How to apply: Test plan 표 한 줄 추가:
```
| integration | Edit a PDF (replace bytes) and re-ingest → new doc_id (different blake3) → `new+=1` for the new doc; old doc unaffected | inline |
```
또는 "동일 path 같은 asset_id 다른 parser_version" 같은 P+ 시나리오는 이번 task 에 포함시키지 않아도 OK — byte 수정 케이스만 추가.
|
||||
- [ ] `docs/SMOKE.md` includes a PDF-fixture step
|
||||
|
||||
## Out of scope
|
||||
|
||||
|
claude-reviewer-01
commented
(칭찬) byte-edit re-ingest 케이스를 P1 idempotency contract 의 다른 axis 로 명시한 게 좋습니다. "identical bytes → identical IDs" 와 "different bytes → different IDs" 두 invariant 가 한 표 안에 나란히 있어야 P1 의 byte-기반 ID 약속이 PDF path 에서도 유지되는 게 확인됩니다. "orphan handling 은 P+" 한 줄까지 단 게 정직 — 이번 PR 의 scope 가 어디서 끝나는지 명확히 표시. (칭찬) byte-edit re-ingest 케이스를 P1 idempotency contract 의 다른 axis 로 명시한 게 좋습니다. "identical bytes → identical IDs" 와 "different bytes → different IDs" 두 invariant 가 한 표 안에 나란히 있어야 P1 의 byte-기반 ID 약속이 PDF path 에서도 유지되는 게 확인됩니다. "orphan handling 은 P+" 한 줄까지 단 게 정직 — 이번 PR 의 scope 가 어디서 끝나는지 명확히 표시.
|
||||
- **RAG `kebab ask` PDF citation** — verifying that `Answer::Citation::source_span = Page { ... }` round-trips through the RAG pipeline is structurally a P4-3 (RAG pipeline) responsibility, not a P7-3 ingest-wiring responsibility. P4-3 already exercises the citation contract over markdown chunks; PDF chunks share the exact same `Citation` shape (the difference is only `source_span` variant). A future PR can bolt on a "PDF chunks survive RAG citation" assertion to either P4-3's existing tests or a dedicated `kebab-rag` integration test — bringing wiremock + RAG fixture infrastructure into `kebab-app` integration tests is out of proportion for the P7-3 invariant (which is "PDF chunks emerge from search with `Page` spans"). Captured here so reviewers can find this decision later.
|
||||
- **Scanned PDF OCR fallback** — empty/extract-failed pages stay searchable=false in v1. A future task ("P+ scanned-PDF-ocr") routes those pages through P6-2's `OllamaVisionOcr` after rasterising the page via a PDF renderer (e.g. `mupdf-rs`, `pdfium-render`). Excluded here because (a) it requires a new system / Rust dep we don't have yet, and (b) v1 user scenario is text-embed PDFs (papers, exported reports).
|
||||
|
claude-reviewer-01
commented
(suggestion / scope) RAG Why: P7-3 implementation PR 의 surface area 를 키워서 review 비용 + RAG 모킹 디버깅 (P6 / P7 implementation 동안 wiremock + RAG 의 multi-turn 행동을 한 번도 fixture 화 한 적 없음) 가 같이 늘어납니다. 단순 How to apply: 해당 행을 두 옵션 중 하나로 변경:
결정 후 hotfixes 추적 부담 없이 갈 수 있는 (A) 가 P6-4 실제 구현 PR 의 scope 와 더 평행. (suggestion / scope) RAG `kebab ask` + mock LLM (wiremock) 테스트가 P7-3 scope 안에서 가장 비용이 큰 항목입니다. 새로운 테스트 인프라 (wiremock 으로 LLM 통신 모킹) 가 `kebab-app` 통합 테스트에 처음 들어오는 것이고, 이건 P4-3 (RAG pipeline) 의 책임 영역을 P7-3 가 떠안는 형국. 본 task 의 진짜 invariant 는 "검색 결과 chunk 의 `source_spans[0]` 가 `Page` 인지" 이지 "RAG 응답이 그걸 cite 하는지" 가 아닙니다 — RAG 가 chunk citation 을 잘 propagate 하는 건 P4-3 의 계약.
Why: P7-3 implementation PR 의 surface area 를 키워서 review 비용 + RAG 모킹 디버깅 (P6 / P7 implementation 동안 wiremock + RAG 의 multi-turn 행동을 한 번도 fixture 화 한 적 없음) 가 같이 늘어납니다. 단순 `kebab search --mode hybrid` 가 PDF chunk + Page span 을 돌려준다는 검증으로 본 task 의 invariant 는 충분.
How to apply: 해당 행을 두 옵션 중 하나로 변경:
- (A, 추천) RAG ask 테스트를 out of scope 로 옮기고 "P9 (UI) 또는 별도 P+ task 가 PDF citation 을 RAG 답변에 surfacing 하는 것을 검증" 로 표시.
- (B) RAG ask 테스트를 유지하되 "opt-in 실제 Ollama 통합 테스트" 로 분리해 hermetic 본 PR 통합 테스트 수트 외부에 둠.
결정 후 hotfixes 추적 부담 없이 갈 수 있는 (A) 가 P6-4 실제 구현 PR 의 scope 와 더 평행.
|
||||
- **Multi-column reading order / table extraction / formula detection / form-field extraction / bookmark or outline ingestion** — all deferred to future PDF-layout task. P7-1 already lists these as out of scope and the wiring inherits.
|
||||
- **Body multilingual via CID font support** — handled at the parser layer (P7-1). UTF-16BE Title metadata works today; non-Latin body text depends on the PDF's font CID mapping.
|
||||
- **Per-medium `chunker_version` config** — current `Config::chunking.chunker_version` is single-valued and serves markdown only. PDF ingest ignores it (hard-codes `pdf-page-v1`). A future P+ task either splits the field per medium or introduces a chunker registry. Logged as a deviation in `tasks/HOTFIXES.md` once implementation lands.
|
||||
- **Chunker dispatch as a runtime registry** — current dispatch is a compile-time `match` on `MediaType`. Adequate while the workspace has 3 chunkers (md-heading-v1, pdf-page-v1, future audio); a registry makes sense once the count grows.
|
||||
- **Parallelism beyond `config.indexing.max_parallel_extractors`** — the existing knob applies. Per-PDF parallelism is a P+ scale-hardening task (a 500-page book produces ≥500 chunks; embedding throughput is the bottleneck, not extraction).
|
||||
- **Progress reporting** (`kebab ingest --progress`) — a 500-page book produces visible-but-silent work; UX gap acknowledged but a P+ enhancement.
|
||||
- **Wire schema bump** — no new wire types; `ingest_report.v1` counters absorb PDF events naturally; `search_hit.v1` already carries `source_span` polymorphically.
|
||||
- **`kebab-store-sqlite` schema migration for `SourceSpan::Page` columns** — the existing `blocks.source_span_kind` / `source_span_payload` columns store the JSON discriminator polymorphically (per P1-6). PDF rows reuse the existing schema without alteration.
|
||||
|
||||
## Risks / notes
|
||||
|
||||
- **`config.chunking.chunker_version` becomes ambiguous.** A user reading their `config.toml` sees `chunker_version = "md-heading-v1"` and reasonably assumes PDFs use the same. They don't. The implementation PR must either log a `tracing::info!` at ingest start ("PDF assets use chunker_version=pdf-page-v1 regardless of config.chunking.chunker_version") OR leave a `TODO` to address in the chunker-registry task. The HOTFIXES entry documents the deviation persistently.
|
||||
- **A 500-page book produces 500+ chunks in one transaction.** The existing `put_chunks` call already loops chunks but the SQLite transaction boundary may need tuning. The implementation PR should benchmark and decide whether to chunk-batch the writes (e.g. 100 chunks per transaction) or trust the existing path. Not a correctness risk — only a throughput / WAL-size risk.
|
||||
- **Encrypted-PDF error message is the only operator-visible signal.** The `kebab ingest` summary shows `errors=1` and the `IngestItem.error` message; a user who didn't read the full output may miss the `qpdf --decrypt` hint. Acceptable in v1 — a future `kebab inspect ingest <run_id>` (P9) renders structured per-asset errors. For now, ensure the test asserts the hint is preserved verbatim from P7-1's bail string.
|
||||
- **Determinism stress with `now()` calls.** Same constraint as P6-4 — extract → chunk pipeline must not insert wall-clock reads between steps. P7-1 emits its own `now()` once for all per-page Provenance events; the PDF wiring branch must add no further `now()`s inside the per-asset path.
|
||||
- **`pdf-page-v1` per-chunk hash variant `#c{char_start}` is opaque.** Downstream tools comparing `chunk_id`s by exact match work fine (it's still a deterministic blake3 input). Tools attempting to derive a stable position from the `chunk_id` alone would fail — they must read `chunk.source_spans[0].char_start`. Documented in P7-2's HOTFIXES entry; cross-referenced here for findability.
|
||||
- **HEIC / RAW image and PDF/A subspecies share `MediaType::Other`.** PDF/A is detected by `kebab-source-fs` as `MediaType::Pdf` (header is still `%PDF-`); PDF subtype variants do not branch separately. Acceptable for v1.
|
||||
|
claude-reviewer-01
commented
(칭찬) RAG ask 검증을 out-of-scope 절에 "P4-3 책임 영역, PDF chunk 도 Citation shape 동일, wiremock+RAG 인프라 신설은 P7-3 invariant 와 비례하지 않음" 으로 옮긴 게 정확한 추론입니다. 본 task 가 검증해야 하는 invariant 는 "PDF chunk 가 search 에서 Page span 으로 나온다" 까지이고, RAG citation 라운드트립은 어떤 chunk 든 변하지 않는 P4-3 의 계약 — out-of-scope 절에 captured 한 추론이 미래 reviewer 가 "왜 P7-3 가 RAG 까지 가지 않았지" 를 묻을 때 답이 됩니다. (칭찬) RAG ask 검증을 out-of-scope 절에 "P4-3 책임 영역, PDF chunk 도 Citation shape 동일, wiremock+RAG 인프라 신설은 P7-3 invariant 와 비례하지 않음" 으로 옮긴 게 정확한 추론입니다. 본 task 가 검증해야 하는 invariant 는 "PDF chunk 가 search 에서 Page span 으로 나온다" 까지이고, RAG citation 라운드트립은 *어떤 chunk* 든 변하지 않는 P4-3 의 계약 — out-of-scope 절에 captured 한 추론이 미래 reviewer 가 "왜 P7-3 가 RAG 까지 가지 않았지" 를 묻을 때 답이 됩니다.
|
||||
(nit / 명시성) Dispatch 절이 "new PDF branch (this task)" 만 적고, 이전에는 PDF 가
_arm 에서Skipped으로 처리됐다는 사실 을 명시하지 않습니다. 본 PR 머지 후IngestReport.skipped카운터가 PDF 자산 수만큼 줄어들고 (MediaType::Audio | MediaType::Other만 남음)scanned/new/updated가 그만큼 늘어나는 운영 변화가 있습니다.Why: P5 eval / 통합 테스트 / 사용자가 ingest 횟수 비교 시 "왜 갑자기 skipped 가 줄었지?" 의 원인을 spec 에서 바로 찾을 수 있어야 함.
How to apply: 첫 번째 bullet 직후 한 줄 추가: