feat(fb-35): verbatim fetch (chunk / doc / span) #126
@@ -168,14 +168,104 @@ fn trim_to_chars(s: &str, n: usize) -> String {
|
||||
}
|
||||
|
||||
fn fetch_span(
|
||||
_app: &App,
|
||||
_id: DocumentId,
|
||||
_line_start: u32,
|
||||
_line_end: u32,
|
||||
_opts: FetchOpts,
|
||||
app: &App,
|
||||
id: DocumentId,
|
||||
line_start: u32,
|
||||
line_end: u32,
|
||||
opts: FetchOpts,
|
||||
) -> Result<FetchResult> {
|
||||
// Implemented in Task 5.
|
||||
anyhow::bail!("fetch_span not yet implemented")
|
||||
let doc = <kebab_store_sqlite::SqliteStore as DocumentStore>::get_document(&app.sqlite, &id)?
|
||||
.ok_or_else(|| {
|
||||
anyhow::Error::new(StructuredError(ErrorV1 {
|
||||
schema_version: ERROR_V1_ID.to_string(),
|
||||
code: "doc_not_found".to_string(),
|
||||
message: format!("doc_id '{}' not found", id.0),
|
||||
details: serde_json::Value::Null,
|
||||
hint: None,
|
||||
}))
|
||||
})?;
|
||||
|
||||
// Reject line-incompatible media types (PDF / audio). `SourceType`
|
||||
// (markdown / note / paper / reference / inbox) is the *user-facing*
|
||||
// category, not the rendering format — the actual byte-level format
|
||||
// lives on the source `RawAsset.media_type`. Look it up via
|
||||
// workspace_path (unique key per asset).
|
||||
if let Some(asset) = <kebab_store_sqlite::SqliteStore as DocumentStore>::get_asset_by_workspace_path(
|
||||
&app.sqlite,
|
||||
&doc.workspace_path,
|
||||
)? {
|
||||
if matches!(
|
||||
asset.media_type,
|
||||
kebab_core::MediaType::Pdf | kebab_core::MediaType::Audio(_)
|
||||
|
|
||||
) {
|
||||
return Err(anyhow::Error::new(StructuredError(ErrorV1 {
|
||||
schema_version: ERROR_V1_ID.to_string(),
|
||||
code: "span_not_supported".to_string(),
|
||||
message: format!(
|
||||
"doc '{}' has media_type {:?}; line-based span fetch unsupported. \
|
||||
Use `fetch chunk` or `fetch doc` instead.",
|
||||
id.0, asset.media_type
|
||||
),
|
||||
details: serde_json::Value::Null,
|
||||
hint: Some("kind = chunk or kind = doc instead".to_string()),
|
||||
})));
|
||||
}
|
||||
}
|
||||
|
||||
if line_start == 0 || line_end == 0 || line_end < line_start {
|
||||
return Err(anyhow::Error::new(StructuredError(ErrorV1 {
|
||||
schema_version: ERROR_V1_ID.to_string(),
|
||||
code: "invalid_input".to_string(),
|
||||
message: format!(
|
||||
"line_start ({line_start}) and line_end ({line_end}) must be 1-based with start <= end"
|
||||
),
|
||||
details: serde_json::Value::Null,
|
||||
hint: None,
|
||||
})));
|
||||
}
|
||||
|
||||
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);
|
||||
|
claude-reviewer-01
commented
버그: 수정안: 또는 **버그**: `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.
|
||||
let lo = (line_start - 1) as usize;
|
||||
let hi = effective_end_raw as usize;
|
||||
let mut text = lines[lo..hi].join("\n");
|
||||
|
||||
let mut truncated = effective_end_raw != line_end;
|
||||
|
claude-reviewer-01
commented
nit: clamp 는 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" 이라 일관성도 맞음.
|
||||
let mut effective_end = effective_end_raw;
|
||||
if let Some(max_tokens) = opts.max_tokens {
|
||||
let max_chars = max_tokens.saturating_mul(4);
|
||||
if text.chars().count() > max_chars {
|
||||
text = trim_to_chars(&text, max_chars);
|
||||
truncated = true;
|
||||
let kept = text.lines().count() as u32;
|
||||
effective_end = (line_start - 1) + kept;
|
||||
}
|
||||
}
|
||||
|
||||
let now = OffsetDateTime::now_utc();
|
||||
let stale = compute_stale(
|
||||
doc_metadata_updated_at(&doc),
|
||||
now,
|
||||
app.config.search.stale_threshold_days,
|
||||
);
|
||||
|
||||
Ok(FetchResult {
|
||||
kind: FetchKind::Span,
|
||||
doc_id: doc.doc_id.clone(),
|
||||
doc_path: doc.workspace_path.clone(),
|
||||
indexed_at: doc_metadata_updated_at(&doc),
|
||||
|
claude-reviewer-01
commented
회차 2 — 회차 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` 으로 보내는 것도 고려해볼 것.
|
||||
stale,
|
||||
chunk: None,
|
||||
context_before: Vec::new(),
|
||||
context_after: Vec::new(),
|
||||
text: Some(text),
|
||||
line_start: Some(line_start),
|
||||
line_end: Some(line_end),
|
||||
effective_end: Some(effective_end),
|
||||
truncated,
|
||||
})
|
||||
}
|
||||
|
||||
/// p9-fb-35: list chunks for a document in ordinal order, return
|
||||
|
||||
@@ -153,3 +153,100 @@ fn fetch_doc_with_max_tokens_truncates() {
|
||||
let text = result.text.expect("doc text");
|
||||
assert!(text.chars().count() <= 100, "trimmed text len {}", text.chars().count());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fetch_span_returns_line_range() {
|
||||
let env = common::TestEnv::new();
|
||||
// Use a list so the canonical-to-markdown roundtrip emits 5
|
||||
// single-line entries joined by `\n` (paragraphs would be joined by
|
||||
// `\n\n`, and CommonMark soft breaks inside one paragraph collapse to
|
||||
// spaces — see crates/kebab-parse-md/src/blocks.rs `Event::SoftBreak`).
|
||||
let body = "- Line one.\n- Line two.\n- Line three.\n- Line four.\n- Line five.\n";
|
||||
common::ingest_md(&env, "lines.md", body);
|
||||
let app = env.app();
|
||||
|
||||
let q = kebab_core::SearchQuery {
|
||||
text: "Line".to_string(),
|
||||
mode: kebab_core::SearchMode::Lexical,
|
||||
k: 1,
|
||||
filters: kebab_core::SearchFilters::default(),
|
||||
};
|
||||
let hits = app.search(q).unwrap();
|
||||
let doc_id = hits[0].doc_id.clone();
|
||||
|
||||
let result = app
|
||||
.fetch(
|
||||
FetchQuery::Span {
|
||||
doc_id,
|
||||
line_start: 2,
|
||||
line_end: 4,
|
||||
},
|
||||
FetchOpts::default(),
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(result.kind, FetchKind::Span);
|
||||
let text = result.text.expect("span text");
|
||||
let line_count = text.lines().count();
|
||||
assert_eq!(line_count, 3, "span should be 3 lines: {text:?}");
|
||||
assert_eq!(result.line_start, Some(2));
|
||||
assert_eq!(result.line_end, Some(4));
|
||||
assert_eq!(result.effective_end, Some(4));
|
||||
assert!(!result.truncated);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fetch_span_clamps_line_end_when_out_of_range() {
|
||||
let env = common::TestEnv::new();
|
||||
common::ingest_md(&env, "short.md", "Line one.\nLine two.\n");
|
||||
let app = env.app();
|
||||
let q = kebab_core::SearchQuery {
|
||||
text: "Line".to_string(),
|
||||
mode: kebab_core::SearchMode::Lexical,
|
||||
k: 1,
|
||||
filters: kebab_core::SearchFilters::default(),
|
||||
};
|
||||
let hits = app.search(q).unwrap();
|
||||
let doc_id = hits[0].doc_id.clone();
|
||||
|
||||
let result = app
|
||||
.fetch(
|
||||
FetchQuery::Span {
|
||||
doc_id,
|
||||
line_start: 1,
|
||||
line_end: 999,
|
||||
},
|
||||
FetchOpts::default(),
|
||||
)
|
||||
.unwrap();
|
||||
let text = result.text.expect("span text");
|
||||
let actual_lines = text.lines().count();
|
||||
assert_eq!(result.effective_end, Some(actual_lines as u32));
|
||||
assert!(actual_lines < 999);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fetch_span_invalid_input_when_zero_lines() {
|
||||
let env = common::TestEnv::new();
|
||||
common::ingest_md(&env, "a.md", "Line one.\n");
|
||||
let app = env.app();
|
||||
let q = kebab_core::SearchQuery {
|
||||
text: "Line".to_string(),
|
||||
mode: kebab_core::SearchMode::Lexical,
|
||||
k: 1,
|
||||
filters: kebab_core::SearchFilters::default(),
|
||||
};
|
||||
let hits = app.search(q).unwrap();
|
||||
let doc_id = hits[0].doc_id.clone();
|
||||
|
||||
let err = app
|
||||
.fetch(
|
||||
FetchQuery::Span {
|
||||
doc_id,
|
||||
line_start: 0,
|
||||
line_end: 0,
|
||||
},
|
||||
FetchOpts::default(),
|
||||
)
|
||||
.unwrap_err();
|
||||
assert!(err.to_string().contains("invalid_input"), "got: {err}");
|
||||
}
|
||||
|
||||
spec §Goal 에는 "PDF / image" 거절이라고 되어있는데 실제 거절은
Pdf | Audio(_)만.MediaType::Image(_)(OCR text doc) 의 line-based span 은 OCR 결과의 의미상 안정성이 낮아서 reject 하는 게 spec 의도로 보임. plan §Mode 동작은 "PDF/audio" 라 두 문서가 어긋남.둘 중 하나로 정리:
Image(_)도span_not_supported추가현재 상태로는 image OCR doc 에 span 호출하면 OCR 텍스트 위에서 동작 — agent 가 안정적인 line 번호를 받기 어려움.
회차 2 — rejection branch 가
Pdf | Audio(_)로만 한정되어 image OCR span fetch 는 normal path 를 그대로 탄다 (회차 1 에서 요구한 "image span 허용" 정책과 일치). spec / SKILL / README / 코드 모두 "PDF / audio" 로 통일되어 round-1 mismatch 해소.