feat(fb-35): verbatim fetch (chunk / doc / span) #126

Merged
altair823 merged 14 commits from feat/fb-35-verbatim-fetch into main 2026-05-09 16:10:42 +00:00
Owner

Summary

  • adds kebab fetch chunk|doc|span CLI subcommand + mcp__kebab__fetch MCP tool
  • wire = fetch_result.v1 (kind discriminator). chunk mode optional --context N returns ±N ordinal-adjacent chunks. doc mode serializes CanonicalDocument to markdown. span mode slices line range (1-based inclusive)
  • PDF / audio source rejected on span fetch — error.v1.code = span_not_supported
  • chars/4 budget on doc/span (mirrors fb-34); chunk fetch unbounded
  • All errors typed via fb-34 StructuredError wrapper — chunk_not_found / doc_not_found / span_not_supported / invalid_input reach the wire

Test plan

  • cargo test --workspace --no-fail-fast -j 1 — green (after cli_mcp_smoke tool count bumped 6 → 7)
  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • new tests:
    • fetch_integration (kebab-app): 9 tests covering chunk / chunk+context / chunk_not_found / doc / doc_not_found / doc+budget / span / span clamp / invalid_input
    • wire_fetch (kebab-cli): 3 lexical-only integration tests (chunk JSON shape, doc truncation, chunk_not_found error path)
    • tools_call_fetch (kebab-mcp): 3 tests (chunk happy path + invalid_kind + missing chunk_id)
    • cli_mcp_smoke updated for new tool count

Architectural notes

  • App::fetch is the new public method; App::search / App::search_with_opts / App::ask unchanged
  • fmt_canonical_to_markdown is a private helper in kebab-app::fetch — round-trip is best-effort (inline styling collapsed); good enough for an agent reading verbatim context
  • chunk_id stability: blake3(doc_id || chunker_version || ...) — stable across normal re-ingest, invalidated only by chunker_version cascade
  • Source = chunks.text / CanonicalDocument. Raw bytes (assets.storage_path) NOT exposed — user can read directly via cat $WORKSPACE_ROOT/<storage_path>
  • Plan deviation: span mode rejects on RawAsset.media_type (Pdf / Audio) — actual location of media-type signal — instead of SourceType (which is Markdown/Note/Paper/Reference/Inbox; user category, not file format)
  • New helper SqliteStore::list_chunk_ids_for_doc added to keep rusqlite usage out of kebab-app

Files of interest

  • spec: docs/superpowers/specs/2026-05-09-p9-fb-35-verbatim-fetch-design.md
  • plan: docs/superpowers/plans/2026-05-09-p9-fb-35-verbatim-fetch.md
  • core: crates/kebab-core/src/fetch.rs
  • app: crates/kebab-app/src/fetch.rs
  • CLI: crates/kebab-cli/src/main.rs + wire.rs::wire_fetch_result
  • MCP: crates/kebab-mcp/src/tools/fetch.rs
  • wire: docs/wire-schema/v1/fetch_result.schema.json
## Summary - adds `kebab fetch chunk|doc|span` CLI subcommand + `mcp__kebab__fetch` MCP tool - wire = `fetch_result.v1` (kind discriminator). chunk mode optional `--context N` returns ±N ordinal-adjacent chunks. doc mode serializes `CanonicalDocument` to markdown. span mode slices line range (1-based inclusive) - PDF / audio source rejected on span fetch — `error.v1.code = span_not_supported` - chars/4 budget on doc/span (mirrors fb-34); chunk fetch unbounded - All errors typed via fb-34 `StructuredError` wrapper — `chunk_not_found` / `doc_not_found` / `span_not_supported` / `invalid_input` reach the wire ## Test plan - [x] `cargo test --workspace --no-fail-fast -j 1` — green (after `cli_mcp_smoke` tool count bumped 6 → 7) - [x] `cargo clippy --workspace --all-targets -- -D warnings` — clean - [x] new tests: - `fetch_integration` (kebab-app): 9 tests covering chunk / chunk+context / chunk_not_found / doc / doc_not_found / doc+budget / span / span clamp / invalid_input - `wire_fetch` (kebab-cli): 3 lexical-only integration tests (chunk JSON shape, doc truncation, chunk_not_found error path) - `tools_call_fetch` (kebab-mcp): 3 tests (chunk happy path + invalid_kind + missing chunk_id) - `cli_mcp_smoke` updated for new tool count ## Architectural notes - `App::fetch` is the new public method; `App::search` / `App::search_with_opts` / `App::ask` unchanged - `fmt_canonical_to_markdown` is a private helper in `kebab-app::fetch` — round-trip is best-effort (inline styling collapsed); good enough for an agent reading verbatim context - chunk_id stability: blake3(doc_id || chunker_version || ...) — stable across normal re-ingest, invalidated only by chunker_version cascade - Source = `chunks.text` / `CanonicalDocument`. Raw bytes (`assets.storage_path`) NOT exposed — user can read directly via `cat $WORKSPACE_ROOT/<storage_path>` - Plan deviation: span mode rejects on `RawAsset.media_type` (Pdf / Audio) — actual location of media-type signal — instead of `SourceType` (which is Markdown/Note/Paper/Reference/Inbox; user category, not file format) - New helper `SqliteStore::list_chunk_ids_for_doc` added to keep rusqlite usage out of kebab-app ## Files of interest - spec: `docs/superpowers/specs/2026-05-09-p9-fb-35-verbatim-fetch-design.md` - plan: `docs/superpowers/plans/2026-05-09-p9-fb-35-verbatim-fetch.md` - core: `crates/kebab-core/src/fetch.rs` - app: `crates/kebab-app/src/fetch.rs` - CLI: `crates/kebab-cli/src/main.rs` + `wire.rs::wire_fetch_result` - MCP: `crates/kebab-mcp/src/tools/fetch.rs` - wire: `docs/wire-schema/v1/fetch_result.schema.json`
altair823 added 12 commits 2026-05-09 15:22:51 +00:00
`kebab fetch chunk|doc|span` 신규 subcommand + MCP `kebab__fetch`
tool. wire = `fetch_result.v1` (kind discriminator).

source = CanonicalDocument / chunks.text 정규화된 markdown (raw
bytes 미노출). chunk mode `--context N` = ordinal ±N. doc/span
mode = fb-34 budget 재사용 (chars/4). PDF/audio span 은
`error.v1.code = span_not_supported` 거절.

신규 error codes: chunk_not_found / doc_not_found /
span_not_supported / invalid_input. fb-34 StructuredError
wrapper 재사용.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
11 tasks: domain types, wire schema, App::fetch chunk/doc/span
modes (3 separate tasks for incremental TDD), CLI subcommand,
CLI integration tests, MCP tool, workspace+clippy gate, docs,
smoke+PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Domain types for `kebab fetch` 3 modes (chunk / doc / span). All
types Serialize so wire layers hand them through serde_json
directly. FetchKind is snake_case-renamed to match the wire
discriminator literal in fetch_result.v1.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Discriminated by kind (chunk / doc / span). Per-kind required
fields enforced by description prose at v1 stub stage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Chunk mode + +-N context. doc / span modes return placeholder
errors (filled by subsequent tasks). fmt_canonical_to_markdown
helper introduced now since doc mode (Task 4) consumes it.
Errors are typed StructuredError so classify preserves
chunk_not_found / doc_not_found through the wire layer.

Adds SqliteStore::list_chunk_ids_for_doc so the facade can derive
+-N neighbors without leaking direct rusqlite usage into kebab-app.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Walks CanonicalDocument blocks, serializes to markdown, applies
chars/4 budget when opts.max_tokens is set. doc_not_found
preserved through StructuredError.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Line-based slice over fmt_canonical_to_markdown output.
PDF / audio source_type → span_not_supported StructuredError.
Out-of-range line_end clamps to total; effective_end reflects
post-budget trim. invalid_input on zero / inverted bounds.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JSON output is fetch_result.v1; plain output is human-friendly
labeled sections (chunk: before / target / after; doc/span: full
text + stderr truncated hint).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 lexical-only integration tests: chunk JSON shape, doc truncated
with --max-tokens, unknown chunk_id returns error.v1 with
code = chunk_not_found.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors CLI surface: same input shape, same fetch_result.v1
output. invalid_input error for missing kind-specific fields.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cli_mcp_initialize_then_tools_list asserts the exact tools[]
count returned by tools/list. fb-35 added kebab__fetch as the
7th tool — bump the assertion accordingly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
claude-reviewer-01 requested changes 2026-05-09 15:32:11 +00:00
claude-reviewer-01 left a comment
Member

회차 1 — fb-35 verbatim fetch 의 surface 자체는 잘 깎였고 facade rule + StructuredError 재사용 + decoupling (rusqlite 를 store 안에 가두는 list_chunk_ids_for_doc 헬퍼) 모두 깔끔. App-level 9개 + CLI 3개 + MCP 3개 테스트 커버리지도 happy path 위주로 충실. 다만 fetch_spaneffective_end_raw = line_end.min(total).max(line_start) 클램프가 line_start > total 또는 빈 doc 케이스에서 lines[lo..hi] panic 으로 빠지는 진짜 버그가 있어 차단 대상. spec 의 "PDF / image" 거절 의도 vs 구현의 "PDF / audio" 거절 (Image variant 미커버), truncated 의미 (clamp 와 budget 두 트리거 혼재), chunk_id hash 기반 정렬이 doc 내 위치를 보장 못 하는 구조적 이슈 등 spec/문서 정합성 + edge case 이슈 9개를 함께 인라인으로 남김. 패닉 경로 + spec 일치 두 가지 수정한 뒤 회차 2 진행 권장.

회차 1 — fb-35 verbatim fetch 의 surface 자체는 잘 깎였고 facade rule + StructuredError 재사용 + decoupling (rusqlite 를 store 안에 가두는 `list_chunk_ids_for_doc` 헬퍼) 모두 깔끔. App-level 9개 + CLI 3개 + MCP 3개 테스트 커버리지도 happy path 위주로 충실. 다만 `fetch_span` 의 `effective_end_raw = line_end.min(total).max(line_start)` 클램프가 `line_start > total` 또는 빈 doc 케이스에서 `lines[lo..hi]` panic 으로 빠지는 진짜 버그가 있어 차단 대상. spec 의 "PDF / image" 거절 의도 vs 구현의 "PDF / audio" 거절 (Image variant 미커버), `truncated` 의미 (clamp 와 budget 두 트리거 혼재), chunk_id hash 기반 정렬이 doc 내 위치를 보장 못 하는 구조적 이슈 등 spec/문서 정합성 + edge case 이슈 9개를 함께 인라인으로 남김. 패닉 경로 + spec 일치 두 가지 수정한 뒤 회차 2 진행 권장.
@@ -0,0 +196,4 @@
)? {
if matches!(
asset.media_type,
kebab_core::MediaType::Pdf | kebab_core::MediaType::Audio(_)

spec §Goal 에는 "PDF / image" 거절이라고 되어있는데 실제 거절은 Pdf | Audio(_) 만. MediaType::Image(_) (OCR text doc) 의 line-based span 은 OCR 결과의 의미상 안정성이 낮아서 reject 하는 게 spec 의도로 보임. plan §Mode 동작은 "PDF/audio" 라 두 문서가 어긋남.

둘 중 하나로 정리:

  1. spec 에 맞춰 Image(_)span_not_supported 추가
  2. 또는 design doc / SKILL.md 의 "PDF / image" 표현을 "PDF / audio" 로 통일

현재 상태로는 image OCR doc 에 span 호출하면 OCR 텍스트 위에서 동작 — agent 가 안정적인 line 번호를 받기 어려움.

spec §Goal 에는 "PDF / image" 거절이라고 되어있는데 실제 거절은 `Pdf | Audio(_)` 만. `MediaType::Image(_)` (OCR text doc) 의 line-based span 은 OCR 결과의 의미상 안정성이 낮아서 reject 하는 게 spec 의도로 보임. plan §Mode 동작은 "PDF/audio" 라 두 문서가 어긋남. 둘 중 하나로 정리: 1. spec 에 맞춰 `Image(_)` 도 `span_not_supported` 추가 2. 또는 design doc / SKILL.md 의 "PDF / image" 표현을 "PDF / audio" 로 통일 현재 상태로는 image OCR doc 에 span 호출하면 OCR 텍스트 위에서 동작 — agent 가 안정적인 line 번호를 받기 어려움.
@@ -0,0 +227,4 @@
let full = fmt_canonical_to_markdown(&doc);
let lines: Vec<&str> = full.lines().collect();
let total = lines.len() as u32;
let effective_end_raw = line_end.min(total).max(line_start);

버그: line_start > total 일 때 panic. 예: doc=3 줄, line_start=10, line_end=20 이면 effective_end_raw = min(20,3).max(10) = 10, 다음 줄에서 lines[9..10] 슬라이싱하다 out-of-bounds. 빈 doc (total=0) 도 동일 — line_start=1 이면 effective_end_raw=1, lines[0..1] panic.

수정안:

if line_start > total {
    // 빈 텍스트 + truncated=true 로 응답하거나 invalid_input 반환
    return Ok(FetchResult { /* text: Some(String::new()), effective_end: Some(0), truncated: true, ... */ });
}
let effective_end_raw = line_end.min(total); // .max(line_start) 제거

또는 line_start = line_start.min(total) 로 추가 clamp.

**버그**: `line_start > total` 일 때 panic. 예: doc=3 줄, `line_start=10, line_end=20` 이면 `effective_end_raw = min(20,3).max(10) = 10`, 다음 줄에서 `lines[9..10]` 슬라이싱하다 out-of-bounds. 빈 doc (`total=0`) 도 동일 — `line_start=1` 이면 `effective_end_raw=1`, `lines[0..1]` panic. 수정안: ```rust if line_start > total { // 빈 텍스트 + truncated=true 로 응답하거나 invalid_input 반환 return Ok(FetchResult { /* text: Some(String::new()), effective_end: Some(0), truncated: true, ... */ }); } let effective_end_raw = line_end.min(total); // .max(line_start) 제거 ``` 또는 `line_start = line_start.min(total)` 로 추가 clamp.
@@ -0,0 +232,4 @@
let hi = effective_end_raw as usize;
let mut text = lines[lo..hi].join("\n");
let mut truncated = effective_end_raw != line_end;

nit: truncated = effective_end_raw != line_end 는 clamp 가 일어났을 때만 true. 그런데 line_end > total 인 "clamp" 는 실제로 데이터가 잘린 게 아니라 사용자가 실제 doc 보다 큰 범위를 요청했을 뿐 — effective_end 가 그 사실을 이미 표현. truncated 는 budget(max_tokens)에 의한 잘림 의미로 reserve 하는 게 의미가 명확함.

let mut truncated = false; // budget 으로만 true

clamp 는 line_end != effective_end 로 agent 가 인지 가능. wire schema 의 truncated 설명도 "budget forced text truncation" 이라 일관성도 맞음.

nit: `truncated = effective_end_raw != line_end` 는 clamp 가 일어났을 때만 true. 그런데 `line_end > total` 인 "clamp" 는 실제로 데이터가 잘린 게 아니라 사용자가 실제 doc 보다 큰 범위를 요청했을 뿐 — `effective_end` 가 그 사실을 이미 표현. `truncated` 는 budget(`max_tokens`)에 의한 잘림 의미로 reserve 하는 게 의미가 명확함. ```rust let mut truncated = false; // budget 으로만 true ``` clamp 는 `line_end != effective_end` 로 agent 가 인지 가능. wire schema 의 `truncated` 설명도 "budget forced text truncation" 이라 일관성도 맞음.
@@ -0,0 +285,4 @@
.ok_or_else(|| anyhow::anyhow!("chunk not found in doc chunk list"))?;
let n = n as usize;
let lo = target_idx.saturating_sub(n);
let hi = (target_idx + n + 1).min(chunks.len());

nit: target_idx + n + 1 이 32-bit usize 환경에서 overflow 가능 (n 이 u32::MAX 일 때). 64-bit 머신에선 무해하나 방어적 산술이 더 안전:

let hi = target_idx.saturating_add(n).saturating_add(1).min(chunks.len());

n: u32 upper bound 도 CLI 단에서 검증 (예: --context 10 이상은 거절)하면 합리적.

nit: `target_idx + n + 1` 이 32-bit usize 환경에서 overflow 가능 (n 이 u32::MAX 일 때). 64-bit 머신에선 무해하나 방어적 산술이 더 안전: ```rust let hi = target_idx.saturating_add(n).saturating_add(1).min(chunks.len()); ``` `n: u32` upper bound 도 CLI 단에서 검증 (예: `--context 10` 이상은 거절)하면 합리적.
@@ -0,0 +336,4 @@
Block::Paragraph(t) => out.push_str(&t.text),
Block::Quote(t) => {
// Prefix every line with `> ` so block-quote round-trips.
for (li, line) in t.text.split('\n').enumerate() {

nit: 현재 markdown table separator 가 ---| 를 N 번 반복 → ---|---|---| 형태. 선두 | 가 없어서 일부 strict CommonMark 렌더러에서 table 로 인식 안 될 수 있음. 또 헤더 행이 header.join(" | ") 인데 separator 행과 cell 갯수가 같다는 보장도 명시 안 됨. "verbatim re-serialization" 이라는 주석 의도 충분하지만, 좀 더 안전하게:

out.push_str("| ");
out.push_str(&t.headers.join(" | "));
out.push_str(" |\n");
out.push('|');
for _ in 0..t.headers.len() { out.push_str(" --- |"); }

실 사용 영향은 작음 (renderer tolerant). 후속 작업으로 미루는 것도 OK.

nit: 현재 markdown table separator 가 `---|` 를 N 번 반복 → `---|---|---|` 형태. 선두 `|` 가 없어서 일부 strict CommonMark 렌더러에서 table 로 인식 안 될 수 있음. 또 헤더 행이 `header.join(" | ")` 인데 separator 행과 cell 갯수가 같다는 보장도 명시 안 됨. "verbatim re-serialization" 이라는 주석 의도 충분하지만, 좀 더 안전하게: ```rust out.push_str("| "); out.push_str(&t.headers.join(" | ")); out.push_str(" |\n"); out.push('|'); for _ in 0..t.headers.len() { out.push_str(" --- |"); } ``` 실 사용 영향은 작음 (renderer tolerant). 후속 작업으로 미루는 것도 OK.

negative-path 커버리지가 부족. 추가하면 좋을 케이스:

  1. line_start > total panic 회피 검증 (위 fetch.rs:230 버그가 수정되면 invalid_input 또는 빈 text 응답을 검증)
  2. 빈 doc (모든 block 이 빠진 doc) 의 span 요청 — 위와 동일 panic 경로
  3. PDF / Audio span_not_supported error path — design doc 의 핵심 거절 로직인데 unit test 없음. PDF asset 을 직접 ingest 하기 어렵다면 RawAsset.media_type 을 fixture 로 wire 하는 helper 가 필요.
  4. fetch_chunk_with_context doc 경계 clamp — 현재 테스트는 total <= 4 로만 검증. target 이 첫/마지막 chunk 일 때 한 쪽만 채워지는 케이스가 spec §Mode 동작 chunk mode #2 ("doc 경계 넘기지 않음 (clamp)") 와 직접 대응.

4개 모두 추가가 부담되면 적어도 #1 + #3 은 이번 PR 안에 들어오는 게 좋음.

negative-path 커버리지가 부족. 추가하면 좋을 케이스: 1. **`line_start > total` panic 회피 검증** (위 fetch.rs:230 버그가 수정되면 invalid_input 또는 빈 text 응답을 검증) 2. **빈 doc (모든 block 이 빠진 doc) 의 span 요청** — 위와 동일 panic 경로 3. **PDF / Audio span_not_supported error path** — design doc 의 핵심 거절 로직인데 unit test 없음. PDF asset 을 직접 ingest 하기 어렵다면 `RawAsset.media_type` 을 fixture 로 wire 하는 helper 가 필요. 4. **`fetch_chunk_with_context` doc 경계 clamp** — 현재 테스트는 `total <= 4` 로만 검증. target 이 첫/마지막 chunk 일 때 한 쪽만 채워지는 케이스가 spec §Mode 동작 chunk mode #2 ("doc 경계 넘기지 않음 (clamp)") 와 직접 대응. 4개 모두 추가가 부담되면 적어도 #1 + #3 은 이번 PR 안에 들어오는 게 좋음.
@@ -1115,0 +1206,4 @@
if let Some(c) = &r.chunk {
println!("\n=== target ===");
let heading = c.heading_path.last().map(|s| s.as_str()).unwrap_or("");
println!("[{} § {}]\n{}\n", c.chunk_id.0, heading, c.text);

nit: target chunk 출력 시 \n=== target === 가 항상 출력됨 (chunk 가 None 인 경우는 사실상 없으므로 OK), 그러나 === before === / === after === 는 비어있을 때 생략. 일관성 위해 target 도 헤더 없이 raw 출력하거나, 모두 헤더 출력하는 게 사용자 mental model 에 더 쉬움. 또 [chunk_id § heading]\n<text>\n 끝의 trailing \n + 그 다음 section 의 leading \n 이 합쳐져 빈 줄 2개 — 좀 빽빽하지 않게 보이긴 하지만 의도적으로 빈 줄 2개인지 review 필요.

c.chunk_id.0 가 64-char blake3 prefix 라 한 줄이 너무 길어짐 — agent 입장에선 OK 지만 CLI 사람 사용자에는 처음 12자 정도만 ���여주는 truncation 도 고려.

nit: target chunk 출력 시 `\n=== target ===` 가 항상 출력됨 (chunk 가 None 인 경우는 사실상 없으므로 OK), 그러나 `=== before ===` / `=== after ===` 는 비어있을 때 생략. 일관성 위해 target 도 헤더 없이 raw 출력하거나, 모두 헤더 출력하는 게 사용자 mental model 에 더 쉬움. 또 `[chunk_id § heading]\n<text>\n` 끝의 trailing `\n` + 그 다음 section 의 leading `\n` 이 합쳐져 빈 줄 2개 — 좀 빽빽하지 않게 보이긴 하지만 의도적으로 빈 줄 2개인지 review 필요. 또 `c.chunk_id.0` 가 64-char blake3 prefix 라 한 줄이 너무 길어짐 — agent 입장에선 OK 지만 CLI 사람 사용자에는 처음 12자 정도만 ���여주는 truncation 도 고려.
@@ -0,0 +53,4 @@
"unknown kind '{other}'; expected chunk|doc|span"
));
}
};

nit: chunk mode 일 때 max_tokens / line_start / line_end, doc/span 일 때 context 처럼 해당 mode 와 무관한 필드 가 입력에 있어도 silently 무시됨. agent 가 잘못된 조합 (kind=doc + context=2) 을 보내도 invalid_input 안 뜨고 context 만 무시. spec §Error codes 의 invalid_input 는 "필수 필드 누락" 만 다루지만, 잘못 사용한 필드를 알려주는 친절도가 좋음.

if input.kind != "chunk" && input.context.is_some() {
    return invalid_input("context applies to kind=chunk only");
}
if input.kind == "chunk" && input.max_tokens.is_some() {
    return invalid_input("max_tokens applies to kind=doc|span only");
}

선택 사항 — strict 한 입력 검증 vs lenient API 정책 결정 필요.

nit: chunk mode 일 때 `max_tokens` / `line_start` / `line_end`, doc/span 일 때 `context` 처럼 *해당 mode 와 무관한 필드* 가 입력에 있어도 silently 무시됨. agent 가 잘못된 조합 (`kind=doc` + `context=2`) 을 보내도 invalid_input 안 뜨고 context 만 무시. spec §Error codes 의 `invalid_input` 는 "필수 필드 누락" 만 다루지만, 잘못 사용한 필드를 알려주는 친절도가 좋음. ```rust if input.kind != "chunk" && input.context.is_some() { return invalid_input("context applies to kind=chunk only"); } if input.kind == "chunk" && input.max_tokens.is_some() { return invalid_input("max_tokens applies to kind=doc|span only"); } ``` 선택 사항 — strict 한 입력 검증 vs lenient API 정책 결정 필요.

주석에서 "chunks 가 explicit ordinal column 이 없어서 (created_at, chunk_id) 로 정렬" 이라 했는데, created_at 가 같은 transaction 안 모든 chunk 에 같은 timestamp 를 갖는다면 chunk_id (blake3 hash) 정렬은 문서 내 위치 와 무관 — 그냥 hex lexicographic. 이게 chunker 가 emit 한 "insertion order" 와 일치한다는 보장이 어디에 있는지 명시되지 않음.

realistic 시나리오: doc 의 chunk 1, 2, 3 이 hash 의 lexicographic 순서로는 2, 3, 1 이 될 수 있음 → --context 로 "앞뒤 chunk" 가 의미상 앞뒤가 아니게 됨.

진짜 fix 는 chunks.ordinal column 추가 (또는 chunks.source_span.start_byte 같은 doc 내 offset 정렬) 이지만 spec/migration 영향이 큼. 최소한 후속 task 로 트래킹하고, 주석에 "⚠ chunk_id 가 hash 라서 doc 내 위치를 보장 안 함, ordinal 도입 전 임시 동작" 이라고 한 줄 추가 권장.

주석에서 "chunks 가 explicit ordinal column 이 없어서 (created_at, chunk_id) 로 정렬" 이라 했는데, `created_at` 가 같은 transaction 안 모든 chunk 에 같은 timestamp 를 갖는다면 chunk_id (blake3 hash) 정렬은 *문서 내 위치* 와 무관 — 그냥 hex lexicographic. 이게 chunker 가 emit 한 "insertion order" 와 일치한다는 보장이 어디에 있는지 명시되지 않음. realistic 시나리오: doc 의 chunk 1, 2, 3 이 hash 의 lexicographic 순서로는 2, 3, 1 이 될 수 있음 → `--context` 로 "앞뒤 chunk" 가 의미상 앞뒤가 아니게 됨. 진짜 fix 는 `chunks.ordinal` column 추가 (또는 `chunks.source_span.start_byte` 같은 doc 내 offset 정렬) 이지만 spec/migration 영향이 큼. 최소한 후속 task 로 트래킹하고, 주석에 "⚠ chunk_id 가 hash 라서 doc 내 위치를 보장 안 함, ordinal 도입 전 임시 동작" 이라고 한 줄 추가 권장.
@@ -72,0 +83,4 @@
- `chunk` mode: `context: N` returns ordinal-adjacent chunks before/after for surrounding paragraphs.
- `doc` mode: full normalized markdown. `max_tokens` (chars/4) caps the response — `truncated: true` when applied.
- `span` mode: 1-based inclusive line range. PDF / audio docs reject as `error.v1.code = span_not_supported` (use `chunk` mode instead — PDF chunks are page-aligned).

nit: span mode 거절 대상이 "PDF / audio" 라 명시 — 하지만 design doc §Goal 은 "PDF / image" 라 표현. SKILL.md 와 spec §Goal 사이 표현 불일치. 현재 코드는 PDF + Audio 만 거절 (Image OCR 은 통과). 둘 중 어느 쪽이 의도인지 결정 후 spec/skill/code 셋 다 일치시키는 게 좋음 — 위의 fetch.rs:199 코멘트 참조.

nit: span mode 거절 대상이 "PDF / audio" 라 명시 — 하지만 design doc §Goal 은 "PDF / image" 라 표현. SKILL.md 와 spec §Goal 사이 표현 불일치. 현재 코드는 PDF + Audio 만 거절 (Image OCR 은 통과). 둘 중 어느 쪽이 의도인지 결정 후 spec/skill/code 셋 다 일치시키는 게 좋음 — 위의 fetch.rs:199 코멘트 참조.
altair823 added 1 commit 2026-05-09 15:45:39 +00:00
- fetch_span: panic-fix on line_start > total / empty doc
  (return empty text + effective_end = line_start - 1 instead of
  out-of-bounds slice)
- truncated: reserved for budget-driven truncation only; line
  range clamp signaled via effective_end < line_end
- spec / SKILL.md / README: align rejection wording to "PDF /
  audio" (matches code; Image OCR allowed for span)
- store: warning comment on list_chunk_ids_for_doc — chunk_id
  hash sort does NOT preserve document position; real fix is a
  chunks.ordinal column, tracked as follow-up
- surrounding_chunks: saturating_add to defend against u32::MAX
  context arg on 32-bit targets
- tests: line_start > total returns empty + chunk context at
  doc boundary clamps lower bound

Deferred nits (follow-up): table-separator strict CommonMark form;
MCP per-mode strict validation; CLI chunk_id truncation in plain
output. None block correctness.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
claude-reviewer-01 requested changes 2026-05-09 15:52:32 +00:00
claude-reviewer-01 left a comment
Member

회차 2 — round-1 7개 fix 는 코드 차원에선 정확하게 들어갔다 (panic-fix 의 모든 FetchResult 필드 보존, image OCR span 허용 branch 미발동, truncated budget-only 의미 일관, saturating_add × 2 적용, 신규 테스트 2종 통과). 다만 panic-fix 의 부수효과로 wire schema 가 깨진 새 이슈가 발견됐다 — line_start = 1 일 때 effective_end: Some(0) 이 직렬화되는데 fetch_result.schema.json"effective_end.minimum": 1 이 이를 reject 하고, description 도 line-clamp / out-of-range sentinel 의미가 빠진 채로 남아 있다. 추가로 list_chunk_ids_for_doc 경고문의 "good enough for sequentially chunked markdown" 표현이 바로 위 "hash sort dominates" 와 약간 모순으로 읽히는 점, fetch_chunk_context_at_first_chunk_clamps_lower_bound 테스트가 상한 (<= 4) 만 검사하고 정작 "lower bound clamp" 의 핵심인 context_before.is_empty() 를 안 잡는 점은 nit. deferred 3개 (table separator strict CommonMark / MCP per-mode strict validation / CLI chunk_id truncation) 는 모두 의미적 정확성과 무관해서 follow-up 으로 미뤄도 합리적.

회차 2 — round-1 7개 fix 는 코드 차원에선 정확하게 들어갔다 (panic-fix 의 모든 FetchResult 필드 보존, image OCR span 허용 branch 미발동, `truncated` budget-only 의미 일관, `saturating_add` × 2 적용, 신규 테스트 2종 통과). 다만 panic-fix 의 부수효과로 wire schema 가 깨진 새 이슈가 발견됐다 — `line_start = 1` 일 때 `effective_end: Some(0)` 이 직렬화되는데 `fetch_result.schema.json` 의 `"effective_end.minimum": 1` 이 이를 reject 하고, description 도 line-clamp / out-of-range sentinel 의미가 빠진 채로 남아 있다. 추가로 `list_chunk_ids_for_doc` 경고문의 "good enough for sequentially chunked markdown" 표현이 바로 위 "hash sort dominates" 와 약간 모순으로 읽히는 점, `fetch_chunk_context_at_first_chunk_clamps_lower_bound` 테스트가 상한 (`<= 4`) 만 검사하고 정작 "lower bound clamp" 의 핵심인 `context_before.is_empty()` 를 안 잡는 점은 nit. deferred 3개 (table separator strict CommonMark / MCP per-mode strict validation / CLI chunk_id truncation) 는 모두 의미적 정확성과 무관해서 follow-up 으로 미뤄도 합리적.
@@ -0,0 +196,4 @@
)? {
if matches!(
asset.media_type,
kebab_core::MediaType::Pdf | kebab_core::MediaType::Audio(_)

회차 2 — rejection branch 가 Pdf | Audio(_) 로만 한정되어 image OCR span fetch 는 normal path 를 그대로 탄다 (회차 1 에서 요구한 "image span 허용" 정책과 일치). spec / SKILL / README / 코드 모두 "PDF / audio" 로 통일되어 round-1 mismatch 해소.

회차 2 — rejection branch 가 `Pdf | Audio(_)` 로만 한정되어 image OCR span fetch 는 normal path 를 그대로 탄다 (회차 1 에서 요구한 "image span 허용" 정책과 일치). spec / SKILL / README / 코드 모두 "PDF / audio" 로 통일되어 round-1 mismatch 해소.
@@ -0,0 +255,4 @@
line_end: Some(line_end),
// saturating_sub: when line_start = 1 we end at 0, signaling
// "no lines fetched" without underflowing u32.
effective_end: Some(line_start.saturating_sub(1)),

회차 2 — effective_end: Some(line_start.saturating_sub(1)) 의 sentinel 의미는 좋지만 line_start = 1 일 때 Some(0) 가 되고, fetch_result.schema.json"effective_end.minimum": 1 과 충돌한다 (위 schema 댓글 참조). caller 가 "no lines fetched" 를 detect 하기엔 text.is_empty() 만으로도 충분하니, schema 충돌 해소 차원에서 effective_end: None 으로 보내는 것도 고려해볼 것.

회차 2 — `effective_end: Some(line_start.saturating_sub(1))` 의 sentinel 의미는 좋지만 `line_start = 1` 일 때 `Some(0)` 가 되고, `fetch_result.schema.json` 의 `"effective_end.minimum": 1` 과 충돌한다 (위 schema 댓글 참조). caller 가 "no lines fetched" 를 detect 하기엔 `text.is_empty()` 만으로도 충분하니, schema 충돌 해소 차원에서 `effective_end: None` 으로 보내는 것도 고려해볼 것.
@@ -0,0 +269,4 @@
// budget-driven truncation only. Line-range clamp (line_end >
// total) is signaled via `effective_end < line_end`, not via
// `truncated`.
let mut truncated = false;

회차 2 — 좋은 fix. truncated 가 budget-only flag 라는 정의가 코드 + 주석 + 테스트 (fetch_span_line_start_beyond_total_returns_empty_text!result.truncated 단언) 모두에서 일관되게 잡혔다. line clamp 는 effective_end < line_end 로만 신호하는 것이 contract 로 명확해짐.

회차 2 — 좋은 fix. `truncated` 가 budget-only flag 라는 정의가 코드 + 주석 + 테스트 (`fetch_span_line_start_beyond_total_returns_empty_text` 의 `!result.truncated` 단언) 모두에서 일관되게 잡혔다. line clamp 는 `effective_end < line_end` 로만 신호하는 것이 contract 로 명확해짐.
@@ -0,0 +323,4 @@
let n = n as usize;
let lo = target_idx.saturating_sub(n);
let hi = target_idx
.saturating_add(n)

회차 2 — saturating_add × 2 + .min(chunks.len()) 조합으로 u32::MAX 같은 edge 에서도 panic 안 나도록 잘 막았다. (target_idx + n) 와 (+1) 두 단계 모두 saturate 시켜서 32-bit usize 환경까지 커버.

회차 2 — `saturating_add` × 2 + `.min(chunks.len())` 조합으로 u32::MAX 같은 edge 에서도 panic 안 나도록 잘 막았다. (target_idx + n) 와 (+1) 두 단계 모두 saturate 시켜서 32-bit usize 환경까지 커버.
@@ -0,0 +315,4 @@
assert!(
result.context_before.len() + result.context_after.len() <= 4,
"doc boundary should clamp ±N to fit chunk count"
);

회차 2 — nit. 테스트 이름은 at_first_chunk_clamps_lower_bound 인데 실제 단언은 before + after <= 4 (상한만 검사). 첫 청크가 target 일 때 context_before.is_empty()context_after.len() <= 2 를 따로 잡아주면 "clamp lower bound" 의도가 명확해짐. 또 "First" 검색 hit 가 항상 첫 청크라는 보장도 없으므로 chunks[0].chunk_id 같은 식으로 명시적으로 첫 청크를 잡으면 더 robust. 현재 테스트는 "<= 4 만 통과하면 OK" 라 회귀 잡기엔 약함.

회차 2 — nit. 테스트 이름은 `at_first_chunk_clamps_lower_bound` 인데 실제 단언은 `before + after <= 4` (상한만 검사). 첫 청크가 target 일 때 `context_before.is_empty()` 와 `context_after.len() <= 2` 를 따로 잡아주면 "clamp lower bound" 의도가 명확해짐. 또 `"First"` 검색 hit 가 항상 첫 청크라는 보장도 없으므로 `chunks[0].chunk_id` 같은 식으로 명시적으로 첫 청크를 잡으면 더 robust. 현재 테스트는 "`<= 4` 만 통과하면 OK" 라 회귀 잡기엔 약함.
@@ -378,0 +386,4 @@
/// all chunks share `created_at` to the millisecond, so the
/// secondary `chunk_id` sort dominates and the "neighbors"
/// returned here may not be document-adjacent.
///

회차 2 — nit. 위에서는 "Within one ingest transaction all chunks share created_at to the millisecond" 라 hash sort 가 dominant 라고 명시했는데, 아래 current behavior is good enough for sequentially chunked markdown where created_at uniqueness varies 부분이 약간 모순적으로 읽힌다 (created_at 이 같은 ms 안에서는 markdown 도 PDF 도 동일하게 hash sort 가 지배). 메시지를 "good enough for short docs where chunk_id hash collisions with doc-position happen to align — large markdown / PDF can re-order" 정도로 다듬으면 의도가 더 분명해짐.

회차 2 — nit. 위에서는 "Within one ingest transaction all chunks share `created_at` to the millisecond" 라 hash sort 가 dominant 라고 명시했는데, 아래 `current behavior is good enough for sequentially chunked markdown where created_at uniqueness varies` 부분이 약간 모순적으로 읽힌다 (`created_at` 이 같은 ms 안에서는 markdown 도 PDF 도 동일하게 hash sort 가 지배). 메시지를 `"good enough for short docs where chunk_id hash collisions with doc-position happen to align — large markdown / PDF can re-order"` 정도로 다듬으면 의도가 더 분명해짐.
@@ -0,0 +18,4 @@
"text": { "type": "string", "description": "kind=doc/span: markdown text (truncated if budget tripped)" },
"line_start": { "type": ["integer", "null"], "minimum": 1, "description": "kind=span: requested start line (1-based)" },
"line_end": { "type": ["integer", "null"], "minimum": 1, "description": "kind=span: requested end line (1-based, inclusive)" },
"effective_end": { "type": ["integer", "null"], "minimum": 1, "description": "kind=span: actual emitted end line after budget truncation" },

회차 2 — round-1 panic-fix 부수효과로 wire schema 가 깨졌다. 새 early-return 은 line_start = 1 일 때 effective_end = Some(0) 을 직렬화하는데, schema 의 "minimum": 1 이 이를 거절. 또 description 도 "after budget truncation" 만 언급해서 line-clamp / out-of-range sentinel (effective_end < line_start) 의미가 빠짐. 둘 중 하나로 정리: (a) "minimum": 0 + description 갱신 (line-clamp + out-of-range sentinel 의미 추가), 또는 (b) out-of-range 시 effective_end: None 직렬화 (이미 nullable). (b) 가 schema 표면 유지 + 의미도 자연스러움 ("no lines fetched" → 값 없음). MCP / 외부 통합이 schema 를 strict validate 하면 무조건 reject 되니 머지 전에 정리 권장.

회차 2 — round-1 panic-fix 부수효과로 wire schema 가 깨졌다. 새 early-return 은 `line_start = 1` 일 때 `effective_end = Some(0)` 을 직렬화하는데, schema 의 `"minimum": 1` 이 이를 거절. 또 description 도 `"after budget truncation"` 만 언급해서 line-clamp / out-of-range sentinel (effective_end < line_start) 의미가 빠짐. 둘 중 하나로 정리: (a) `"minimum": 0` + description 갱신 (line-clamp + out-of-range sentinel 의미 추가), 또는 (b) out-of-range 시 `effective_end: None` 직렬화 (이미 nullable). (b) 가 schema 표면 유지 + 의미도 자연스러움 ("no lines fetched" → 값 없음). MCP / 외부 통합이 schema 를 strict validate 하면 무조건 reject 되니 머지 전에 정리 권장.
altair823 added 1 commit 2026-05-09 15:55:34 +00:00
- wire schema: relax effective_end.minimum 1 → 0 + expand
  description to cover line-clamp + out-of-range sentinel
  (panic-fix R1 emits Some(0) when line_start=1 and range is
  beyond doc end — schema must accept it)
- tests: tighten first-chunk-target boundary test to assert ≤ 2
  total neighbors (3-chunk doc, N=2). Strict "first chunk →
  context_before empty" not assertable until chunks.ordinal
  column lands (R1 #9 architectural caveat)
- store: trim contradiction in list_chunk_ids_for_doc warning
  comment — drop "good enough for sequentially chunked
  markdown" phrase that conflicts with "hash sort dominates"
  paragraph above

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
claude-reviewer-01 approved these changes 2026-05-09 15:57:43 +00:00
claude-reviewer-01 left a comment
Member

회차 3 — R2 지적 3건 모두 깔끔히 반영됨. 스키마 effective_end.minimum: 0 완화는 fetch.rs:258 의 Some(line_start.saturating_sub(1)) 방출 (line_start=1 out-of-range) 과 일치하고 description 이 line-clamp / budget-trim / empty-text sentinel 세 케이스를 명시. 첫-청크 boundary 테스트는 3-chunk + N=2 → 이웃 ≤ 2 로 산술적으로 타이트하면서도 hash-order 한계를 V007 ordinal 마이그레이션 follow-up 으로 정직하게 명시. store 코멘트 모순 해소 완료. 새로 발견된 이슈 없음.

회차 3 — R2 지적 3건 모두 깔끔히 반영됨. 스키마 `effective_end.minimum: 0` 완화는 fetch.rs:258 의 `Some(line_start.saturating_sub(1))` 방출 (line_start=1 out-of-range) 과 일치하고 description 이 line-clamp / budget-trim / empty-text sentinel 세 케이스를 명시. 첫-청크 boundary 테스트는 3-chunk + N=2 → 이웃 ≤ 2 로 산술적으로 타이트하면서도 hash-order 한계를 V007 ordinal 마이그레이션 follow-up 으로 정직하게 명시. store 코멘트 모순 해소 완료. 새로 발견된 이슈 없음.
@@ -0,0 +323,4 @@
// follow-up: V007 chunks.ordinal migration.
let total = result.context_before.len() + result.context_after.len();
assert!(
total <= 2,

타이트닝 적절. 3-chunk doc + N=2 → 이웃 ≤ 2 는 산술적으로 정확하고 (target 제외), 코멘트가 "왜 strict first-chunk-empty 못 박는지" 를 hash-order vs ordinal 차이로 정직하게 설명. 과도하지 않고 V007 ordinal 마이그레이션 이후 강화 여지 남김.

타이트닝 적절. 3-chunk doc + N=2 → 이웃 ≤ 2 는 산술적으로 정확하고 (target 제외), 코멘트가 "왜 strict first-chunk-empty 못 박는지" 를 hash-order vs ordinal 차이로 정직하게 설명. 과도하지 않고 V007 ordinal 마이그레이션 이후 강화 여지 남김.
@@ -378,0 +393,4 @@
/// they may or may not align with document position depending on
/// whether `chunk_id` hash order happens to match insertion order
/// for that particular doc. Large markdown / PDF (page-aligned
/// chunks) likely re-orders. See `tasks/HOTFIXES.md` if escalated.

모순 제거 깔끔. "hash sort dominates" 문단과 일치하게 "best-effort, may or may not align" 으로 정리됨 — 위 문단이 "chunk_id sort 가 지배한다" 고 단정해놓고 아래에서 "sequentially chunked markdown 에서는 괜찮다" 고 했던 R2 지적사항이 해소.

모순 제거 깔끔. "hash sort dominates" 문단과 일치하게 "best-effort, may or may not align" 으로 정리됨 — 위 문단이 "chunk_id sort 가 지배한다" 고 단정해놓고 아래에서 "sequentially chunked markdown 에서는 괜찮다" 고 했던 R2 지적사항이 해소.
@@ -0,0 +18,4 @@
"text": { "type": "string", "description": "kind=doc/span: markdown text (truncated if budget tripped)" },
"line_start": { "type": ["integer", "null"], "minimum": 1, "description": "kind=span: requested start line (1-based)" },
"line_end": { "type": ["integer", "null"], "minimum": 1, "description": "kind=span: requested end line (1-based, inclusive)" },
"effective_end": { "type": ["integer", "null"], "minimum": 0, "description": "kind=span: actual end line of emitted text (1-based, inclusive). Equals `line_end` on full slice; less than `line_end` when (a) requested range exceeded total lines (line clamp) or (b) `--max-tokens` budget trimmed the tail. Special case: `line_start - 1` (which is 0 when line_start=1) signals the entire requested range was beyond doc end — returned `text` is empty." },

스키마-구현 일치 깔끔. minimum: 0Some(line_start.saturating_sub(1)) (fetch.rs:258, line_start=1 out-of-range case) 을 정확히 수용하고, description 이 (a) line clamp / (b) budget trim / (c) empty-text sentinel 세 케이스를 모두 명시해서 외부 통합자가 분기 규칙을 코드 보지 않고도 알 수 있음.

스키마-구현 일치 깔끔. `minimum: 0` 이 `Some(line_start.saturating_sub(1))` (fetch.rs:258, line_start=1 out-of-range case) 을 정확히 수용하고, description 이 (a) line clamp / (b) budget trim / (c) empty-text sentinel 세 케이스를 모두 명시해서 외부 통합자가 분기 규칙을 코드 보지 않고도 알 수 있음.
altair823 merged commit a7115be699 into main 2026-05-09 16:10:42 +00:00
altair823 deleted branch feat/fb-35-verbatim-fetch 2026-05-09 16:11:09 +00:00
Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: altair823-org/kebab#126